Author SHA1 Message Date
sneak d199ff53ce Harden the lint-guard shell scanner against silent evasions (closes #121)
check / check (pull_request) Failing after 1s
The guard test's shell scanner was weaker than its commit message
claimed. Two holes are closed.

shellCode now treats `<<` as a here-document only when it is a real
redirection: outside single and double quotes, not past an unquoted
word-initial `#` that begins an inline comment, and followed by a
delimiter word. A `<<` inside a quoted string or an inline comment no
longer opens a phantom here-document that swallows the rest of the
file -- including the silent case where the fake terminator recurs
later as a line of its own -- and a here-document still open at end of
file is a loud error rather than a silent truncation.

assertLinterIsContainerised now cuts the joined line into the simple
commands the shell would run -- on `;`, `&&`, `||` and `|` -- and
requires the command that names the linter to begin with docker. So
`docker info; golangci-lint run` and `docker info || golangci-lint run`
are rejected, while script/lint-fix's `docker run ... golangci-lint`
still passes.

The scanner comment now names the inline-comment exception alongside
the quoted-string and arithmetic ones, and the inherent limits of a
text scan. Dockerfile.lint's citation is corrected from `lll` to
`revive`, the finding the recorded evidence actually named.

model: claude-opus-4-8
2026-09-21 17:34:41 +00:00
29 changed files with 920 additions and 1176 deletions
+25 -18
View File
@@ -20,21 +20,33 @@ jobs:
# check.yml runs script/cibuild, which does all of its work inside # check.yml runs script/cibuild, which does all of its work inside
# the digest-pinned Dockerfile images -- so without this step the # the digest-pinned Dockerfile images -- so without this step the
# release either fails at the before-hook or, worse, ships binaries # release either fails at the before-hook or, worse, ships binaries
# built by whatever Go the runner happens to carry. # built by whatever unpinned Go the runner happens to carry.
# REPO_POLICIES.md requires every external reference to be pinned,
# and script/release already refuses a goreleaser that is not the
# pinned build; the compiler that actually produces the artifacts
# is the last thing that should be exempt from that.
# #
# actions/setup-go would pin the action by commit sha, but the Go # go-version-file rather than a literal: go.mod's `go 1.26.1` is
# tarball it downloads at runtime is verified against no value in # the single source of truth for the toolchain, the same way the
# this repo, and the action exposes no checksum input. # Dockerfile FROM line is the single source of truth for the
# REPO_POLICIES.md requires every external reference to be pinned # linter version that script/lint enforces. It is a three-component
# by hash with no exceptions, and this is the compiler that # version, so setup-go resolves it exactly -- no silent drift onto
# produces the published binaries -- the input where a substituted # a newer patch release.
# artifact matters most. So Go is installed the way goreleaser is: #
# script/install-go downloads the exact archive for go.mod's `go` # actions/setup-go v5.6.0, 2025-12-15. Pinned by commit sha, like
# directive and refuses it unless its sha256 matches the value # the checkout above. v5.x is a node20 action, matching the node20
# committed in the script, then puts .tool/go/bin on PATH for the # actions/checkout v4 already in use here; the v6/v7 line requires
# steps below. # a node24 runner, which this Gitea runner has never been asked
# for and cannot be assumed to provide.
- name: Install Go - name: Install Go
run: script/install-go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff
with:
go-version-file: go.mod
# setup-go's module cache needs a runner-side cache backend.
# A release is cut rarely and a cold module download costs
# seconds; a release failing because a cache service is absent
# costs a re-tag. Off, deliberately.
cache: false
- name: Install goreleaser - name: Install goreleaser
run: script/install-goreleaser run: script/install-goreleaser
- name: Release - name: Release
@@ -46,8 +58,3 @@ jobs:
# It is deliberately not the runner's automatic token, which is # It is deliberately not the runner's automatic token, which is
# not guaranteed to carry that scope. # not guaranteed to carry that scope.
GITEA_TOKEN: ${{ secrets.RELEASE_TOKEN }} GITEA_TOKEN: ${{ secrets.RELEASE_TOKEN }}
# Build with the toolchain install-go just verified, never a
# different one auto-downloaded from a `toolchain` directive:
# the point of the hash pin is that this exact compiler makes
# the release.
GOTOOLCHAIN: local
+3 -5
View File
@@ -22,10 +22,8 @@ FROM golang:1.26.1-alpine@sha256:2389ebfa5b7f43eeafbd6be0c3700cc46690ef842ad962f
ARG VERSION=dev ARG VERSION=dev
# Build tooling: make, plus a C toolchain because `go test -race` needs cgo. # Install build dependencies for CGO (mattn/go-sqlite3) and sqlite3 CLI (tests)
# The sqlite driver is pure Go (modernc.org/sqlite), so no sqlite library or RUN apk add --no-cache make build-base sqlite
# CLI is required.
RUN apk add --no-cache make build-base
WORKDIR /src WORKDIR /src
@@ -73,7 +71,7 @@ RUN CGO_ENABLED=0 go build -ldflags "-X 'sneak.berlin/go/vaultik/internal/global
# alpine:3.21, 2026-02-25 # alpine:3.21, 2026-02-25
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709 FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
RUN apk add --no-cache ca-certificates RUN apk add --no-cache ca-certificates sqlite
# Copy binary from builder # Copy binary from builder
COPY --from=builder /vaultik /usr/local/bin/vaultik COPY --from=builder /vaultik /usr/local/bin/vaultik
+4 -12
View File
@@ -251,8 +251,7 @@ local index alone, and still exits zero.
per-snapshot-name (`--keep-latest` keeps the latest of each name, not the per-snapshot-name (`--keep-latest` keeps the latest of each name, not the
latest globally). latest globally).
* `--keep-latest`: Keep only the most recent snapshot of each name * `--keep-latest`: Keep only the most recent snapshot of each name
* `--older-than <duration>`: Remove snapshots older than duration (e.g. `30d`, * `--older-than <duration>`: Remove snapshots older than duration (e.g. `30d`, `6m`, `1y`)
`4w`, `6mo`, `1y`; `m` is minutes, `mo` is months)
* `--snapshot <name>`: Restrict to specific snapshot names (repeat for multiple) * `--snapshot <name>`: Restrict to specific snapshot names (repeat for multiple)
* `--force`: Skip confirmation prompt * `--force`: Skip confirmation prompt
@@ -614,6 +613,7 @@ regardless of color setting (emoji are not color).
and the pre-commit hook both run it. A `golangci-lint` installed on and the pre-commit hook both run it. A `golangci-lint` installed on
`PATH` is not a substitute and is never used on a host, whatever its `PATH` is not a substitute and is never used on a host, whatever its
version. version.
* `sqlite3` CLI, which the test suite shells out to
* S3-compatible object storage (or local filesystem, or rclone remote) * S3-compatible object storage (or local filesystem, or rclone remote)
## development workflow ## development workflow
@@ -644,8 +644,8 @@ standard: normalized scripts in `script/` are the entrypoints for the
development workflow, and the Makefile targets are thin shims that call development workflow, and the Makefile targets are thin shims that call
them. We provide: them. We provide:
* `script/bootstrap` — install all development dependencies (go, Go * `script/bootstrap` — install all development dependencies (go, sqlite3,
module download). It deliberately does not install `golangci-lint`; Go module download). It deliberately does not install `golangci-lint`;
see `script/lint` below. see `script/lint` below.
* `script/setup` — make a fresh clone ready for development: runs * `script/setup` — make a fresh clone ready for development: runs
`script/bootstrap`, then `script/install-precommit` `script/bootstrap`, then `script/install-precommit`
@@ -659,14 +659,6 @@ them. We provide:
called by `script/bootstrap`; the release workflow calls it directly called by `script/bootstrap`; the release workflow calls it directly
because it needs `goreleaser` but not the Docker daemon because it needs `goreleaser` but not the Docker daemon
`script/bootstrap` insists on. `script/bootstrap` insists on.
* `script/install-go` — install the Go toolchain named by `go.mod`'s
`go` directive into `.tool/go` from a sha256-verified `go.dev`
archive, and put it on `PATH`. Idempotent. Called only by the release
workflow, which needs a host Go for `goreleaser` to shell out to;
nothing else on the release runner does. `actions/setup-go` is not
used because it verifies the downloaded toolchain against no value in
this repo. Bumping Go edits `go.mod`, the checksum in this script, and
the `Dockerfile` `golang` digest together.
* `script/release` — cross-compile and publish the release artifacts * `script/release` — cross-compile and publish the release artifacts
with the pinned `goreleaser`. Refuses a `goreleaser` on `PATH` whose with the pinned `goreleaser`. Refuses a `goreleaser` on `PATH` whose
version is not the pinned one, on the same reasoning as `script/lint`. version is not the pinned one, on the same reasoning as `script/lint`.
+5 -82
View File
@@ -25,88 +25,12 @@ release" is exactly the contradiction
# Completed Steps # Completed Steps
- 2026-09-21: Stopped `prune` from reporting a failed row count as 0
([issue #96](https://git.eeqj.de/sneak/vaultik/issues/96)). The seven
`getTableCount` reads in `PruneDatabase` discarded their error, so a
query that could not run became a plausible `0` and the before/after
delta computed from it looked like real work. Each read now logs at
warn on failure and renders as `unknown`, never `0`, so an empty table
is distinguishable from one that could not be queried. The counts have
no `--json` representation — under `--json` the summary is suppressed
entirely — so nothing there can show a false `0`.
- 2026-09-21: Stopped `--json` from silencing stderr diagnostics
([issue #112](https://git.eeqj.de/sneak/vaultik/issues/112)). `--json`
used to be folded into `Quiet`, which pinned the log level to `WARN`,
so `prune --json` gave a machine consumer no record of the local index
rows it deleted even under `--verbose`. `--json` now quiets only the
stdout UI (the JSON document must stay clean, per
[issue #108](https://git.eeqj.de/sneak/vaultik/issues/108)); the stderr
log level follows `--verbose`/`--debug` again. The coupling was
removed the same way for `snapshot verify`, `snapshot remove`, and
`remote info`, which carried it for the same outdated reason.
- 2026-09-21: Made the s3 storage backend report a missing object as
`storage.ErrNotFound`, like the `file` and `rclone` backends and as the
`Storer` interface documents. `S3Storer.Get` and `Stat` returned the raw
AWS SDK error, so `errors.Is(err, storage.ErrNotFound)` was false on s3
and callers branched differently per backend. Added a small `s3.IsNotFound`
helper (reused by `HeadObject`) and a test that a missing key maps to
`ErrNotFound`
([issue #129](https://git.eeqj.de/sneak/vaultik/issues/129)).
- 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as
corrupt. Its final blob-integrity check hashed the encrypted
downloaded bytes with a single SHA256 and compared that to the blob
ID, which is the double SHA256 of the plaintext, so the two could
never match. It now hashes the decompressed plaintext and compares the
double SHA256. Added a test that backs up a real snapshot, deep-verifies
it, then flips a byte in one stored blob and confirms deep verification
then fails
([issue #131](https://git.eeqj.de/sneak/vaultik/issues/131)).
- 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata
database through the `modernc.org/sqlite` driver instead of shelling
out to the external `sqlite` command-line binary (issue #120). A
backup no longer needs that binary on `PATH`, so `make check` passes
on a stock `go install` host; `script/bootstrap` and the `Dockerfile`
(both the test-build and the shipped runtime stage) no longer install
it, and a new test asserts the uploaded database keeps no pages from
deleted rows. Dropped the now-false note on the 2026-08-07 entry below
that said bootstrap installs it.
- 2026-09-21: Made `.gitea/workflows/check.yml` run on pushes to `main` - 2026-09-21: Made `.gitea/workflows/check.yml` run on pushes to `main`
and `next` and on pull requests against either, so unit PRs (whose and `next` and on pull requests against either, so unit PRs (whose
base is `next`) and `next` itself get a CI run instead of relying on a base is `next`) and `next` itself get a CI run instead of relying on a
local `make check` local `make check`
([issue #122](https://git.eeqj.de/sneak/vaultik/issues/122)). ([issue #122](https://git.eeqj.de/sneak/vaultik/issues/122)).
- 2026-09-21: Hash-verified the Go toolchain in the release workflow
([issue #105](https://git.eeqj.de/sneak/vaultik/issues/105)). New
`script/install-go` downloads the exact `go.dev` archive for `go.mod`'s
`go` directive and refuses it unless its sha256 matches a value
committed in the script; `.gitea/workflows/release.yml` calls it
instead of `actions/setup-go`, which verified the downloaded toolchain
against nothing in the repo. `GOTOOLCHAIN: local` on the release step
keeps that exact compiler from auto-switching. Bumping Go now touches
`go.mod`, the checksum, and the `Dockerfile` `golang` digest together.
- 2026-09-21: Collapsed the two duration parsers into one and fixed the
`--older-than` months example
([issue #123](https://git.eeqj.de/sneak/vaultik/issues/123)). Two
functions named `parseDuration` existed with different grammars;
`snapshot purge --older-than` and `--keep-newer-than` both already went
through the one in `internal/vaultik`, while the richer copy in
`internal/cli/duration.go` was reachable only from its own test. Kept
the live-path parser and deleted the unused one, so no flag's accepted
grammar changes. The trap the issue was filed over: `README.md`
documented `6m` as the months example for `--older-than`, but `m` is
minutes, so the documented command deleted every snapshot older than
six minutes on a destructive flag. Corrected the doc to `6mo` and put
both flags' help text on one example list that states `m` is minutes
and `mo` is months. The surviving parser now rejects negatives, which
it previously accepted (`-5h`) or silently made positive (`-5d`).
Table-driven tests cover every unit, `6m` as six minutes, `6mo` as 180
days, and rejection of a bare number, an unknown unit, and a negative.
- 2026-08-10: Moved every lint run into its own container, as a build - 2026-08-10: Moved every lint run into its own container, as a build
step ([issue #113](https://git.eeqj.de/sneak/vaultik/issues/113)). step ([issue #113](https://git.eeqj.de/sneak/vaultik/issues/113)).
New root `Dockerfile.lint`, built by `script/lint`, runs New root `Dockerfile.lint`, built by `script/lint`, runs
@@ -135,11 +59,10 @@ release" is exactly the contradiction
into each check command, and a fresh `$(date +%s%N)$$` per invocation into each check command, and a fresh `$(date +%s%N)$$` per invocation
computed as a bare assignment. `cmd/vaultik/lintdocker_test.go` computed as a bare assignment. `cmd/vaultik/lintdocker_test.go`
parses both Dockerfiles and both scripts and fails if any part of parses both Dockerfiles and both scripts and fails if any part of
that is dropped, because every way of losing it is silent. No test that is dropped, because every way of losing it is silent. Its
asserts that no script runs the host linter: `script/lint` is the one host-lint assertion is structural — no script runs `golangci-lint`
lint entry point and runs `golangci-lint` only inside the container, except through `docker` — rather than a search for the one retired
and keeping it that way is a review matter, not something a test variable name, which nothing could ever reintroduce.
proves.
The product `Dockerfile` lost its lint stage rather than gaining a The product `Dockerfile` lost its lint stage rather than gaining a
second linter pin: `make lint` is now `docker build`, so the stage second linter pin: `make lint` is now `docker build`, so the stage
@@ -606,7 +529,7 @@ release" is exactly the contradiction
was green was wrong. was green was wrong.
- 2026-08-07: Added the standard `.golangci.yml` and `.editorconfig` - 2026-08-07: Added the standard `.golangci.yml` and `.editorconfig`
(issue #59); lint findings under the new config are tracked in issue (issue #59); lint findings under the new config are tracked in issue
#61. #61. `script/bootstrap` now installs sqlite3 (needed by tests).
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section Makefile shims, README Entrypoints section
- 2026-07-02: Consolidated CLI verbs, retired overlapping commands; bound - 2026-07-02: Consolidated CLI verbs, retired overlapping commands; bound
+333 -7
View File
@@ -1,6 +1,8 @@
package main_test package main_test
import ( import (
"errors"
"fmt"
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
@@ -28,11 +30,6 @@ import (
// -- that a real finding actually fails the build -- is verified by // -- that a real finding actually fails the build -- is verified by
// hand against a deliberately broken tree, recorded on the pull // hand against a deliberately broken tree, recorded on the pull
// request. // request.
//
// One property is deliberately NOT tested here: that no script runs the
// linter on the host. script/lint is the only lint entry point, and it
// runs golangci-lint only inside the container; keeping it that way is a
// review matter, not something a test in this file establishes.
// The files under guard, relative to the repository root. // The files under guard, relative to the repository root.
const ( const (
@@ -42,8 +39,9 @@ const (
cibuildScript = "script/cibuild" cibuildScript = "script/cibuild"
) )
// linterBinary is the linter's command name, used to locate the // linterBinary is the linter's command name. Every occurrence of it in
// config-verify and lint steps in Dockerfile.lint. // executable shell in this repo must be inside a docker invocation; see
// TestNoHostLintPathRemains.
const linterBinary = "golangci-lint" const linterBinary = "golangci-lint"
// checkEpochARG is the declaration, with no default value. A default // checkEpochARG is the declaration, with no default value. A default
@@ -223,6 +221,199 @@ func TestCibuildBuildsBothDockerfilesWithFreshEpochs(t *testing.T) {
"%s must build %s", cibuildScript, lintDockerfile) "%s must build %s", cibuildScript, lintDockerfile)
} }
// TestNoHostLintPathRemains fails if any escape hatch to a host linter
// comes back. The owner's ruling is that every lint run happens inside
// a container; a PATH binary that happens to match the pinned version
// is a different build reached by a different code path, and admitting
// it is what lets a local pass disagree with CI.
//
// This asserts the PROPERTY -- no script invokes the linter except
// through docker -- rather than the absence of any particular variable
// name. An earlier version of this test looked only for the literal
// VAULTIK_LINT_IN_CONTAINER, the name of the hatch that was removed
// alongside it, so nothing could ever trip it again: a hatch under any
// other name left it passing. A structural test that passes on a broken
// tree is worse than no test, because it is what a later reader trusts
// instead of re-deriving the invariant.
//
// script/lint-fix is not exempted. It is the one script that runs the
// linter as a container rather than as a build step, but it still runs
// it in one, so the same property holds of it.
func TestNoHostLintPathRemains(t *testing.T) {
t.Parallel()
root := repoRoot(t)
entries, err := os.ReadDir(filepath.Join(root, "script"))
require.NoError(t, err)
require.NotEmpty(t, entries, "no scripts found to scan")
for _, entry := range entries {
if entry.IsDir() {
continue
}
name := filepath.Join("script", entry.Name())
lines, err := shellCode(readRepoFile(t, name))
require.NoError(t, err, "scanning %s", name)
for _, line := range lines {
assertLinterIsContainerised(t, name, line)
}
}
}
// assertLinterIsContainerised fails unless every command that names the
// linter on this joined line is a docker command. Merely mentioning
// docker somewhere on the line is not enough; see linterRunsInDocker.
func assertLinterIsContainerised(t *testing.T, name, line string) {
t.Helper()
assert.True(t, linterRunsInDocker(line),
"%s runs %s outside a container; every command that names the"+
" linter must begin with docker (line: %s)", name, linterBinary,
line)
}
// linterRunsInDocker reports whether the linter, wherever it appears on
// this joined shell line, is only ever the argument of a docker command.
// The line is cut into the simple commands the shell would run -- on
// `;`, `&&`, `||` and `|` -- and every command that names the linter
// must begin with `docker`. This is what distinguishes the one
// legitimate invocation, script/lint-fix's `docker run ... golangci-lint
// run ...`, from evasions like `docker info; golangci-lint run` or
// `docker info || golangci-lint run`, where the linter sits in a command
// of its own that docker does not introduce.
func linterRunsInDocker(line string) bool {
for _, command := range splitShellCommands(line) {
if !strings.Contains(command, linterBinary) {
continue
}
if !strings.HasPrefix(strings.TrimSpace(command), "docker") {
return false
}
}
return true
}
// splitShellCommands breaks a joined shell line into the separate simple
// commands the shell would run, cutting at the `;`, `&&`, `||` and `|`
// operators (`||` before `|`, so the two-character operator is not split
// twice). It is deliberately blind to quoting and to `$(...)`: no line
// under guard puts one of these operators inside a string, and a scan
// that tried to account for that would be the kind of half-parser this
// file avoids.
func splitShellCommands(line string) []string {
for _, op := range []string{"&&", "||", "|", ";"} {
line = strings.ReplaceAll(line, op, "\n")
}
return strings.Split(line, "\n")
}
// TestShellCodeSeesCodeAndNotProse keeps the scanner above honest. It
// has to ignore comments and here-document bodies, because script/lint
// and script/bootstrap both NAME golangci-lint in prose -- in comments,
// and in the error text they print -- precisely to say that the host
// binary is never used. A scanner that went blind, by over-eager
// stripping or by failing to join continuation lines, would make
// TestNoHostLintPathRemains pass on everything.
func TestShellCodeSeesCodeAndNotProse(t *testing.T) {
t.Parallel()
// A `<<` inside quotes is not a here-document, so the code after it
// is still scanned; a real `<<EOF` opens one and its body is dropped.
script := strings.Join([]string{
"#!/bin/sh",
"# a comment naming golangci-lint",
"cat >&2 <<EOF",
"prose naming golangci-lint, printed not executed",
"EOF",
`echo "a left shift << is not a here-document"`,
"docker run --rm \\",
" \"$image\" \\",
" golangci-lint run ./...",
}, "\n")
lines, err := shellCode(script)
require.NoError(t, err)
assert.Equal(t,
[]string{
"cat >&2 <<EOF",
`echo "a left shift << is not a here-document"`,
`docker run --rm "$image" golangci-lint run ./...`,
},
lines)
// A `<<` inside an inline comment is not a here-document either: an
// unquoted, word-initial `#` begins a comment that runs to end of
// line, so the `<< STOP` is prose. The code that follows is still
// scanned -- here, a host golangci-lint that TestNoHostLintPathRemains
// must then see rather than have swallowed. The fake terminator even
// recurs later as a line of its own; a phantom here-document would
// swallow everything up to it silently, past the end-of-file error
// that only catches a terminator which never recurs.
inlineComment := strings.Join([]string{
": # housekeeping marker << STOP",
"golangci-lint run --config .golangci.yml ./...",
"STOP",
}, "\n")
lines, err = shellCode(inlineComment)
require.NoError(t, err)
assert.Equal(t,
[]string{
": # housekeeping marker << STOP",
"golangci-lint run --config .golangci.yml ./...",
"STOP",
},
lines)
// A here-document still open at end of file must be a loud error,
// not a silent truncation of everything the scanner has yet to see.
unterminated := strings.Join([]string{
"cat <<EOF",
"body line naming golangci-lint, no terminator follows",
}, "\n")
_, err = shellCode(unterminated)
require.Error(t, err)
}
// TestLinterCommandMustBeginWithDocker pins the property that a mention
// of docker somewhere on the line is not enough: the command that
// actually runs the linter has to be a docker command. The two evasions
// from the issue place the linter in a command of its own, joined to a
// harmless docker command by `;` or `||`; both must be rejected. The
// containerised invocation script/lint-fix writes -- docker run with the
// linter as its argument -- must still be accepted.
func TestLinterCommandMustBeginWithDocker(t *testing.T) {
t.Parallel()
rejected := []string{
"docker info >/dev/null; golangci-lint run ./...",
"docker info || golangci-lint run ./...",
"docker build . && golangci-lint run ./... | tee log",
}
for _, line := range rejected {
assert.False(t, linterRunsInDocker(line),
"a linter command docker does not introduce must be rejected: %s",
line)
}
accepted := []string{
`docker run --rm "$image" golangci-lint run ./...`,
`docker run --rm --user x --volume "$ROOT:/src" img golangci-lint run --fix ./...`,
}
for _, line := range accepted {
assert.True(t, linterRunsInDocker(line),
"a docker-introduced linter command must be accepted: %s", line)
}
}
// assertEpochExpandedInto fails unless some instruction runs the named // assertEpochExpandedInto fails unless some instruction runs the named
// command with the epoch expanded into it. Expansion, not mere // command with the epoch expanded into it. Expansion, not mere
// declaration: an ARG that no instruction references is not guaranteed // declaration: an ARG that no instruction references is not guaranteed
@@ -327,6 +518,141 @@ func indexContaining(found []string, want string) int {
return -1 return -1
} }
// shellCode returns a POSIX shell script's executable lines: comments
// dropped, here-document bodies dropped, and backslash continuations
// joined so a multi-line command is a single string. Whitespace is
// collapsed, as it is for Dockerfile instructions. A here-document left
// open at end of file is an error rather than a silent truncation of
// everything after its opener.
//
// Both exclusions are load-bearing rather than tidiness. The scripts
// name golangci-lint in prose to state that the host binary is never
// used, and joining continuations is what lets the one legitimate
// container invocation -- script/lint-fix's `docker run`, whose linter
// command sits several lines below the word `docker` -- be recognised
// as containerised.
//
// This is a text scan, not a shell: it cannot see a linter name
// assembled at runtime, one split across a continuation, a script in a
// subdirectory of script/, or anything in the Makefile.
func shellCode(contents string) ([]string, error) {
var (
out []string
joined string
terminate string
)
for line := range strings.SplitSeq(contents, "\n") {
trimmed := strings.TrimSpace(line)
if terminate != "" {
if trimmed == terminate {
terminate = ""
}
continue
}
if joined == "" && (trimmed == "" || strings.HasPrefix(trimmed, "#")) {
continue
}
joined += strings.TrimSuffix(trimmed, `\`) + " "
if strings.HasSuffix(trimmed, `\`) {
continue
}
joined = strings.Join(strings.Fields(joined), " ")
terminate = heredocTerminator(joined)
out = append(out, joined)
joined = ""
}
if terminate != "" {
return nil, fmt.Errorf("%w: terminator %q", errUnterminatedHeredoc,
terminate)
}
return out, nil
}
// errUnterminatedHeredoc is what shellCode returns when a here-document
// is still open at end of file. Its callers require its absence, so an
// unterminated body -- which would otherwise be swallowed silently --
// fails the guard loudly.
var errUnterminatedHeredoc = errors.New(
"here-document opened but never closed before end of file")
// heredocTerminator returns the delimiter word of the here-document the
// command opens, or "" if it opens none. A `<<` only opens one when it
// is a real redirection: outside single and double quotes, not in an
// inline comment, and followed by a delimiter word. A `<<` inside a
// quoted string, past an unquoted word-initial `#` (which begins a
// comment that runs to end of line), or in an arithmetic left shift
// like `$((x << 2))`, is not a here-document; the first two are cases
// this guards, the last appears in no script here. Only the first
// opener on a line is recognised; nothing in script/ opens two.
func heredocTerminator(line string) string {
var quote byte // 0 when outside quotes, else '\'' or '"'
for i := 0; i+1 < len(line); i++ {
c := line[i]
switch {
case quote != 0:
if c == quote {
quote = 0
}
case c == '\'' || c == '"':
quote = c
case c == '#' && (i == 0 || line[i-1] == ' '):
// A word-initial `#` starts a comment; the rest of the
// line, `<<` included, is prose, not a redirection.
return ""
case c == '<' && line[i+1] == '<':
return heredocWord(line[i+2:])
}
}
return ""
}
// heredocWord extracts the delimiter that follows `<<` or `<<-`: it drops
// an optional `-`, skips blanks, then reads the delimiter -- quoted or
// bare -- and returns it with quotes removed. `<<-'EOF'` and `<< EOF`
// both yield "EOF". It returns "" when no word follows, so a bare `<<`
// opens nothing.
func heredocWord(after string) string {
after = strings.TrimLeft(strings.TrimPrefix(after, "-"), " \t")
var (
word strings.Builder
quote byte
)
for i := range len(after) {
c := after[i]
switch {
case quote != 0:
if c == quote {
quote = 0
} else {
word.WriteByte(c)
}
case c == '\'' || c == '"':
quote = c
case c == ' ' || c == '\t':
return word.String()
default:
word.WriteByte(c)
}
}
return word.String()
}
// readRepoFile reads a file by its path relative to the repository // readRepoFile reads a file by its path relative to the repository
// root. // root.
func readRepoFile(t *testing.T, name string) string { func readRepoFile(t *testing.T, name string) string {
+1 -1
View File
@@ -192,7 +192,7 @@ Tracks blob upload metrics.
After a snapshot is completed: After a snapshot is completed:
1. Copy database to temporary file 1. Copy database to temporary file
2. Clean temporary database to contain only current snapshot data 2. Clean temporary database to contain only current snapshot data
3. VACUUM the trimmed database so deleted rows leave no pages behind 3. Export to SQL dump using sqlite3
4. Compress with zstd and encrypt with age 4. Compress with zstd and encrypt with age
5. Upload to S3 as `metadata/{remote-key}/db.zst.age` 5. Upload to S3 as `metadata/{remote-key}/db.zst.age`
6. Generate blob manifest and upload as `metadata/{remote-key}/manifest.json.zst` 6. Generate blob manifest and upload as `metadata/{remote-key}/manifest.json.zst`
+4 -11
View File
@@ -48,11 +48,6 @@ type AppOptions struct {
// silenced — per the documented convention that --quiet suppresses // silenced — per the documented convention that --quiet suppresses
// non-error output only. The startup banner is printed by Entry // non-error output only. The startup banner is printed by Entry
// before cobra parses arguments, gated by the same arg-level check. // before cobra parses arguments, gated by the same arg-level check.
//
// --json quiets the UI here too, because stdout then carries a JSON
// document and human narration would corrupt it. Unlike Quiet it does
// not lower the stderr log level (issue #112), so --verbose/--debug
// still surface diagnostics alongside the document.
func setupGlobals( func setupGlobals(
lc fx.Lifecycle, g *globals.Globals, v *vaultik.Vaultik, opts log.Options, lc fx.Lifecycle, g *globals.Globals, v *vaultik.Vaultik, opts log.Options,
) { ) {
@@ -60,7 +55,7 @@ func setupGlobals(
OnStart: func(_ context.Context) error { OnStart: func(_ context.Context) error {
g.StartTime = time.Now().UTC() g.StartTime = time.Now().UTC()
if opts.Cron || opts.Quiet || opts.JSON { if opts.Cron || opts.Quiet {
v.UI.SetQuiet(true) v.UI.SetQuiet(true)
} }
@@ -207,10 +202,9 @@ func RunApp(ctx context.Context, app *fx.App) error {
// instance in a goroutine, report a failure prefixed with failMsg // instance in a goroutine, report a failure prefixed with failMsg
// (suppressed while suppressErrors is true, e.g. under --json), then // (suppressed while suppressErrors is true, e.g. under --json), then
// trigger shutdown. The operation is cancelled when the app stops. // trigger shutdown. The operation is cancelled when the app stops.
// jsonOutput marks a command whose stdout is a JSON document: it quiets // extraQuiet is OR-ed into LogOptions.Quiet (e.g. --json output modes).
// the UI but, unlike Quiet, leaves the stderr log level alone.
func runVaultikApp( func runVaultikApp(
cmd *cobra.Command, jsonOutput, suppressErrors bool, cmd *cobra.Command, extraQuiet, suppressErrors bool,
failMsg string, op func(v *vaultik.Vaultik) error, failMsg string, op func(v *vaultik.Vaultik) error,
) error { ) error {
configPath, err := ResolveConfigPath() configPath, err := ResolveConfigPath()
@@ -225,8 +219,7 @@ func runVaultikApp(
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet || extraQuiet,
JSON: jsonOutput,
}, },
Modules: []fx.Option{}, Modules: []fx.Option{},
Invokes: []fx.Option{ Invokes: []fx.Option{
+126
View File
@@ -0,0 +1,126 @@
package cli
import (
"errors"
"fmt"
"regexp"
"strconv"
"strings"
"time"
)
// Approximate lengths of the extended calendar units accepted by
// parseDuration.
const (
durationDay = 24 * time.Hour
durationWeek = 7 * durationDay
durationMonth = 30 * durationDay
durationYear = 365 * durationDay
)
var (
errNegativeDuration = errors.New("negative durations are not supported")
errInvalidDuration = errors.New("invalid duration format")
errUnknownTimeUnit = errors.New("unknown time unit")
)
// parseDuration parses duration strings. Supports standard Go duration format
// (e.g., "3h30m", "1h45m30s") as well as extended units:
// - d: days (e.g., "30d", "7d")
// - w: weeks (e.g., "2w", "4w")
// - mo: months (30 days) (e.g., "6mo", "1mo")
// - y: years (365 days) (e.g., "1y", "2y")
//
// Can combine units: "1y6mo", "2w3d", "1d12h30m"
func parseDuration(s string) (time.Duration, error) {
// First try standard Go duration parsing
d, err := time.ParseDuration(s)
if err == nil {
return d, nil
}
// Extended duration parsing
// Check for negative values
if strings.HasPrefix(strings.TrimSpace(s), "-") {
return 0, errNegativeDuration
}
// Pattern matches: number + unit, repeated
re := regexp.MustCompile(`(\d+(?:\.\d+)?)\s*([a-zA-Z]+)`)
matches := re.FindAllStringSubmatch(s, -1)
if len(matches) == 0 {
return 0, fmt.Errorf("%w: %q", errInvalidDuration, s)
}
var total time.Duration
for _, match := range matches {
valueStr := match[1]
unit := strings.ToLower(match[2])
value, err := strconv.ParseFloat(valueStr, 64)
if err != nil {
return 0, fmt.Errorf("invalid number %q: %w", valueStr, err)
}
d, err := durationForUnit(value, unit)
if err != nil {
return 0, err
}
total += d
}
return total, nil
}
// durationForUnit converts a value with a (case-normalized) unit suffix
// into a time.Duration, accepting Go's standard units plus the extended
// calendar units.
func durationForUnit(value float64, unit string) (time.Duration, error) {
switch unit {
// Standard time units
case "ns", "nanosecond", "nanoseconds":
return time.Duration(value), nil
case "us", "µs", "microsecond", "microseconds":
return time.Duration(value * float64(time.Microsecond)), nil
case "ms", "millisecond", "milliseconds":
return time.Duration(value * float64(time.Millisecond)), nil
case "s", "sec", "second", "seconds":
return time.Duration(value * float64(time.Second)), nil
case "m", "min", "minute", "minutes":
return time.Duration(value * float64(time.Minute)), nil
case "h", "hr", "hour", "hours":
return time.Duration(value * float64(time.Hour)), nil
// Extended units
case "d", "day", "days":
return time.Duration(value * float64(durationDay)), nil
case "w", "week", "weeks":
return time.Duration(value * float64(durationWeek)), nil
case "mo", "month", "months":
// Using 30 days as approximation
return time.Duration(value * float64(durationMonth)), nil
case "y", "year", "years":
// Using 365 days as approximation
return time.Duration(value * float64(durationYear)), nil
default:
// Try parsing as standard Go duration unit
testStr := "1" + unit
_, err := time.ParseDuration(testStr)
if err != nil {
return 0, fmt.Errorf("%w: %q", errUnknownTimeUnit, unit)
}
// It's a valid Go duration unit, parse the full value
fullStr := fmt.Sprintf("%g%s", value, unit)
d, err := time.ParseDuration(fullStr)
if err != nil {
return 0, fmt.Errorf("invalid duration %q: %w", fullStr, err)
}
return d, nil
}
}
+299
View File
@@ -0,0 +1,299 @@
package cli //nolint:testpackage // needs access to unexported parseDuration
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type parseDurationCase struct {
name string
input string
expected time.Duration
wantErr bool
}
// runParseDurationCases executes a table of parseDuration cases as
// parallel subtests.
func runParseDurationCases(t *testing.T, tests []parseDurationCase) {
t.Helper()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := parseDuration(tt.input)
if tt.wantErr {
require.Error(t, err, "expected error for input %q", tt.input)
return
}
require.NoError(t, err, "unexpected error for input %q", tt.input)
assert.Equal(t, tt.expected, got, "duration mismatch for input %q", tt.input)
})
}
}
func TestParseDurationStandard(t *testing.T) {
t.Parallel()
runParseDurationCases(t, []parseDurationCase{
{
name: "standard seconds",
input: "30s",
expected: 30 * time.Second,
},
{
name: "standard minutes",
input: "45m",
expected: 45 * time.Minute,
},
{
name: "standard hours",
input: "2h",
expected: 2 * time.Hour,
},
{
name: "standard combined",
input: "3h30m",
expected: 3*time.Hour + 30*time.Minute,
},
{
name: "standard complex",
input: "1h45m30s",
expected: 1*time.Hour + 45*time.Minute + 30*time.Second,
},
{
name: "standard with milliseconds",
input: "1s500ms",
expected: 1*time.Second + 500*time.Millisecond,
},
})
}
func TestParseDurationExtendedUnits(t *testing.T) {
t.Parallel()
runParseDurationCases(t, []parseDurationCase{
// Extended units - days
{
name: "single day",
input: "1d",
expected: 24 * time.Hour,
},
{
name: "multiple days",
input: "7d",
expected: 7 * 24 * time.Hour,
},
{
name: "fractional days",
input: "1.5d",
expected: 36 * time.Hour,
},
{
name: "days spelled out",
input: "3days",
expected: 3 * 24 * time.Hour,
},
// Extended units - weeks
{
name: "single week",
input: "1w",
expected: 7 * 24 * time.Hour,
},
{
name: "multiple weeks",
input: "4w",
expected: 4 * 7 * 24 * time.Hour,
},
{
name: "weeks spelled out",
input: "2weeks",
expected: 2 * 7 * 24 * time.Hour,
},
// Extended units - months
{
name: "single month",
input: "1mo",
expected: 30 * 24 * time.Hour,
},
{
name: "multiple months",
input: "6mo",
expected: 6 * 30 * 24 * time.Hour,
},
{
name: "months spelled out",
input: "3months",
expected: 3 * 30 * 24 * time.Hour,
},
// Extended units - years
{
name: "single year",
input: "1y",
expected: 365 * 24 * time.Hour,
},
{
name: "multiple years",
input: "2y",
expected: 2 * 365 * 24 * time.Hour,
},
{
name: "years spelled out",
input: "1year",
expected: 365 * 24 * time.Hour,
},
})
}
func TestParseDurationCombinedAndErrors(t *testing.T) {
t.Parallel()
runParseDurationCases(t, []parseDurationCase{
// Combined extended units
{
name: "weeks and days",
input: "2w3d",
expected: 2*7*24*time.Hour + 3*24*time.Hour,
},
{
name: "years and months",
input: "1y6mo",
expected: 365*24*time.Hour + 6*30*24*time.Hour,
},
{
name: "days and hours",
input: "1d12h",
expected: 24*time.Hour + 12*time.Hour,
},
{
name: "complex combination",
input: "1y2mo3w4d5h6m7s",
expected: 365*24*time.Hour + 2*30*24*time.Hour +
3*7*24*time.Hour + 4*24*time.Hour +
5*time.Hour + 6*time.Minute + 7*time.Second,
},
{
name: "with spaces",
input: "1d 12h 30m",
expected: 24*time.Hour + 12*time.Hour + 30*time.Minute,
},
// Edge cases
{
name: "zero duration",
input: "0s",
expected: 0,
},
{
name: "large duration",
input: "10y",
expected: 10 * 365 * 24 * time.Hour,
},
// Error cases
{
name: "empty string",
input: "",
wantErr: true,
},
{
name: "invalid format",
input: "abc",
wantErr: true,
},
{
name: "unknown unit",
input: "5x",
wantErr: true,
},
{
name: "invalid number",
input: "xyzd",
wantErr: true,
},
{
name: "negative not supported",
input: "-5d",
wantErr: true,
},
})
}
func TestParseDurationSpecialCases(t *testing.T) {
t.Parallel()
// Test that standard Go durations work exactly as expected
standardDurations := []string{
"300ms",
"1.5h",
"2h45m",
"72h",
"1us",
"1µs",
"1ns",
}
for _, d := range standardDurations {
expected, err := time.ParseDuration(d)
require.NoError(t, err)
got, err := parseDuration(d)
require.NoError(t, err)
assert.Equal(t, expected, got, "standard duration %q should parse identically", d)
}
}
func TestParseDurationRealWorldExamples(t *testing.T) {
t.Parallel()
// Test real-world snapshot purge scenarios
tests := []struct {
description string
input string
olderThan time.Duration
}{
{
description: "keep snapshots from last 30 days",
input: "30d",
olderThan: 30 * 24 * time.Hour,
},
{
description: "keep snapshots from last 6 months",
input: "6mo",
olderThan: 6 * 30 * 24 * time.Hour,
},
{
description: "keep snapshots from last year",
input: "1y",
olderThan: 365 * 24 * time.Hour,
},
{
description: "keep snapshots from last week and a half",
input: "1w3d",
olderThan: 10 * 24 * time.Hour,
},
{
description: "keep snapshots from last 90 days",
input: "90d",
olderThan: 90 * 24 * time.Hour,
},
}
for _, tt := range tests {
t.Run(tt.description, func(t *testing.T) {
t.Parallel()
got, err := parseDuration(tt.input)
require.NoError(t, err)
assert.Equal(t, tt.olderThan, got)
// Verify the duration makes sense for snapshot purging
assert.Greater(t, got, time.Hour,
"snapshot purge duration should be at least an hour")
})
}
}
@@ -1,139 +0,0 @@
package cli //nolint:testpackage // shares the prune fixtures and capture helpers
import (
"bytes"
"io"
"os"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// staleRecordLogMessage is the local-cleanup audit line CleanupLocalSnapshots
// logs for each stale record. It is exactly the signal issue #112 says a
// machine consumer lost under --json: gated off stdout, and pinned below
// the log level on stderr because --json used to force Quiet.
const staleRecordLogMessage = "Removing stale local snapshot record"
// TestEntryPruneJSONStderrHonoursVerbosity is the end-to-end regression
// guard for issue #112. Under --json the log level must still follow
// --verbose/--debug rather than being pinned to WARN, so the
// local-cleanup records reach stderr under --verbose while stdout stays
// exactly one JSON document; without --verbose they stay below the
// level, as they do without --json.
//
// Both halves are asserted together on the same run, because the fix has
// to keep the document clean (issue #108) while freeing stderr.
//
// Not parallel: it replaces os.Args, os.Stdout, os.Stderr and the xdg
// globals.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntryPruneJSONStderrHonoursVerbosity(t *testing.T) {
for _, testCase := range []struct {
name string
verbose bool
wantOnStderr bool
}{
{
name: "verbose json surfaces the cleanup record on stderr",
verbose: true,
wantOnStderr: true,
},
{
name: "json alone keeps the cleanup record below the level",
verbose: false,
wantOnStderr: false,
},
} {
t.Run(testCase.name, func(t *testing.T) {
configPath := writeHermeticPruneConfig(t, true)
previousArgs := os.Args
t.Cleanup(func() {
os.Args = previousArgs
rootFlags = RootFlags{}
})
args := []string{
programName, flagConfig, configPath, cmdPrune, flagJSON,
}
if testCase.verbose {
args = append(args, "--verbose")
}
os.Args = args
stdout, stderr := captureProcessStdoutAndStderr(t, Entry)
// The document stays clean in both cases: freeing stderr must
// not regress issue #108.
requireExactlyOneJSONDocument(t, stdout)
if testCase.wantOnStderr {
assert.Contains(t, stderr, staleRecordLogMessage,
"--verbose --json must emit the cleanup record on stderr")
assert.Contains(t, stderr, stalePruneSnapshotID,
"the record must name the snapshot it removed")
} else {
assert.NotContains(t, stderr, staleRecordLogMessage,
"without --verbose the record stays below the log level")
}
})
}
}
// captureProcessStdoutAndStderr redirects both of the process's own
// standard streams to pipes for the duration of fn and returns what was
// written to each. The redirection is at the file-descriptor level
// because the logger binds os.Stderr when it initializes inside fn, and
// the JSON document reaches os.Stdout independently; the point is to see
// where each actually lands.
//
// Not parallel-safe: os.Stdout and os.Stderr are process-global.
func captureProcessStdoutAndStderr(t *testing.T, fn func()) (string, string) {
t.Helper()
outReader, outWriter, err := os.Pipe()
require.NoError(t, err)
errReader, errWriter, err := os.Pipe()
require.NoError(t, err)
previousOut, previousErr := os.Stdout, os.Stderr
os.Stdout, os.Stderr = outWriter, errWriter
capturedOut := drain(outReader)
capturedErr := drain(errReader)
fn()
os.Stdout, os.Stderr = previousOut, previousErr
require.NoError(t, outWriter.Close())
require.NoError(t, errWriter.Close())
out, errOut := <-capturedOut, <-capturedErr
require.NoError(t, outReader.Close())
require.NoError(t, errReader.Close())
return out, errOut
}
// drain copies a reader to a string on a goroutine and delivers the
// result once the writer end is closed.
func drain(reader io.Reader) <-chan string {
captured := make(chan string, 1)
go func() {
var buf bytes.Buffer
_, _ = io.Copy(&buf, reader)
captured <- buf.String()
}()
return captured
}
+1 -2
View File
@@ -46,8 +46,7 @@ work (e.g. after a crashed backup or to reclaim storage).`,
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet || opts.JSON,
JSON: opts.JSON,
}, },
Modules: []fx.Option{}, Modules: []fx.Option{},
Invokes: []fx.Option{ Invokes: []fx.Option{
+1 -2
View File
@@ -88,8 +88,7 @@ func newRemoteInfoCommand() *cobra.Command {
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet || jsonOutput,
JSON: jsonOutput,
}, },
Modules: []fx.Option{}, Modules: []fx.Option{},
Invokes: []fx.Option{ Invokes: []fx.Option{
+3 -6
View File
@@ -141,8 +141,7 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
"orphaned blobs") "orphaned blobs")
cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "", cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "",
"With --prune: keep snapshots newer than this duration "+ "With --prune: keep snapshots newer than this duration "+
"(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months) "+ "(e.g. 4w, 30d, 6mo) instead of only the latest")
"instead of only the latest")
return cmd return cmd
} }
@@ -205,8 +204,7 @@ restrict the operation to specific snapshot names.`,
cmd.Flags().BoolVar(&opts.KeepLatest, "keep-latest", false, cmd.Flags().BoolVar(&opts.KeepLatest, "keep-latest", false,
"Keep only the latest snapshot of each name") "Keep only the latest snapshot of each name")
cmd.Flags().StringVar(&opts.OlderThan, "older-than", "", cmd.Flags().StringVar(&opts.OlderThan, "older-than", "",
"Remove snapshots older than duration "+ "Remove snapshots older than duration (e.g., 30d, 6m, 1y)")
"(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months)")
cmd.Flags().BoolVar(&opts.Force, "force", false, "Skip confirmation prompt") cmd.Flags().BoolVar(&opts.Force, "force", false, "Skip confirmation prompt")
cmd.Flags().StringArrayVar(&opts.Names, "snapshot", nil, cmd.Flags().StringArrayVar(&opts.Names, "snapshot", nil,
"Restrict to snapshots with these names (repeat for multiple)") "Restrict to snapshots with these names (repeat for multiple)")
@@ -239,8 +237,7 @@ func newSnapshotVerifyCommand() *cobra.Command {
LogOptions: log.Options{ LogOptions: log.Options{
Verbose: rootFlags.Verbose, Verbose: rootFlags.Verbose,
Debug: rootFlags.Debug, Debug: rootFlags.Debug,
Quiet: rootFlags.Quiet, Quiet: rootFlags.Quiet || opts.JSON,
JSON: opts.JSON,
}, },
Modules: []fx.Option{}, Modules: []fx.Option{},
Invokes: []fx.Option{ Invokes: []fx.Option{
+1 -15
View File
@@ -14,18 +14,8 @@ var Module = fx.Module("log",
) )
// New creates a new logger configuration from provided options. // New creates a new logger configuration from provided options.
//
// JSON is intentionally not carried into Config: a command emitting a
// JSON document on stdout must keep its stderr log level under
// --verbose/--debug, so --json must not lower it (issue #112). JSON
// silences the stdout UI in setupGlobals instead.
func New(opts Options) Config { func New(opts Options) Config {
return Config{ return Config(opts)
Verbose: opts.Verbose,
Debug: opts.Debug,
Cron: opts.Cron,
Quiet: opts.Quiet,
}
} }
// Options are provided by the CLI. // Options are provided by the CLI.
@@ -34,8 +24,4 @@ type Options struct {
Debug bool Debug bool
Cron bool Cron bool
Quiet bool Quiet bool
// JSON marks a command whose stdout carries a machine-readable
// document. It silences the human UI on stdout (see setupGlobals),
// but unlike Quiet it leaves the stderr log level alone.
JSON bool
} }
+5 -13
View File
@@ -219,7 +219,11 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
Key: aws.String(fullKey), Key: aws.String(fullKey),
}) })
if err != nil { if err != nil {
if IsNotFound(err) { var (
notFound *s3types.NotFound
noSuchKey *s3types.NoSuchKey
)
if errors.As(err, &notFound) || errors.As(err, &noSuchKey) {
return false, nil return false, nil
} }
@@ -229,18 +233,6 @@ func (c *Client) HeadObject(ctx context.Context, key string) (bool, error) {
return true, nil return true, nil
} }
// IsNotFound reports whether err indicates that an object does not exist.
// Head and Get requests surface a missing object as different SDK types,
// so both are checked here.
func IsNotFound(err error) bool {
var (
notFound *s3types.NotFound
noSuchKey *s3types.NoSuchKey
)
return errors.As(err, &notFound) || errors.As(err, &noSuchKey)
}
// ObjectInfo contains information about an S3 object. // ObjectInfo contains information about an S3 object.
// It is used by ListObjectsStream to return object metadata // It is used by ListObjectsStream to return object metadata
// along with any errors encountered during listing. // along with any errors encountered during listing.
+5 -21
View File
@@ -44,6 +44,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"os/exec"
"path/filepath" "path/filepath"
"strings" "strings"
"time" "time"
@@ -668,31 +669,14 @@ func (sm *SnapshotManager) collectCleanupStats(
// vacuumDatabase runs VACUUM on the database to remove deleted data and compact // vacuumDatabase runs VACUUM on the database to remove deleted data and compact
// This is critical for security - ensures no stale/deleted data pages are uploaded // This is critical for security - ensures no stale/deleted data pages are uploaded
//
// VACUUM runs through the modernc.org/sqlite driver, on a freshly opened
// connection with no transaction in flight (VACUUM cannot run inside one).
// The database opens in WAL mode, so VACUUM's rewrite lands in the WAL; the
// checkpoint on Close flushes it into the main file, which is the file we
// then compress and upload.
func (sm *SnapshotManager) vacuumDatabase(ctx context.Context, dbPath string) error { func (sm *SnapshotManager) vacuumDatabase(ctx context.Context, dbPath string) error {
log.Debug("Running VACUUM on database", "path", dbPath) log.Debug("Running VACUUM on database", "path", dbPath)
//nolint:gosec // G204: fixed argv; dbPath is our own temp file path
cmd := exec.CommandContext(ctx, "sqlite3", dbPath, "VACUUM;")
db, err := database.New(ctx, dbPath) output, err := cmd.CombinedOutput()
if err != nil { if err != nil {
return fmt.Errorf("opening database for VACUUM: %w", err) return fmt.Errorf("running VACUUM: %w (output: %s)", err, string(output))
}
defer func() {
cerr := db.Close()
if cerr != nil {
log.Debug("Failed to close database after VACUUM",
"path", dbPath, "error", cerr)
}
}()
_, err = db.ExecWithLog(ctx, "VACUUM")
if err != nil {
return fmt.Errorf("running VACUUM: %w", err)
} }
return nil return nil
-92
View File
@@ -2,7 +2,6 @@
package snapshot package snapshot
import ( import (
"bytes"
"context" "context"
"database/sql" "database/sql"
"io" "io"
@@ -97,97 +96,6 @@ func verifyCleanedDB(
} }
} }
// TestVacuumDatabaseRemovesDeletedData proves the export path uploads a
// compacted database: after rows carrying a recognizable marker are deleted
// and vacuumDatabase runs, no page holding that marker survives in the file
// on disk (the file compressFile later reads for upload).
func TestVacuumDatabaseRemovesDeletedData(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dbPath := filepath.Join(tempDir, "snapshot.db")
db, err := database.New(ctx, dbPath)
if err != nil {
t.Fatalf("failed to create database: %v", err)
}
// A marker distinctive enough that its presence in the raw file can only
// come from the rows inserted below.
marker := []byte("VACUUM_PROBE_DEADBEEF_DELETED_ROW")
payload := bytes.Repeat(marker, 128) // ~4 KiB per row
_, err = db.Conn().ExecContext(ctx,
"CREATE TABLE vacuum_probe (id INTEGER PRIMARY KEY, payload BLOB)")
if err != nil {
t.Fatalf("failed to create probe table: %v", err)
}
for range 512 {
_, err = db.Conn().ExecContext(ctx,
"INSERT INTO vacuum_probe (payload) VALUES (?)", payload)
if err != nil {
t.Fatalf("failed to insert probe row: %v", err)
}
}
_, err = db.Conn().ExecContext(ctx, "DELETE FROM vacuum_probe")
if err != nil {
t.Fatalf("failed to delete probe rows: %v", err)
}
// Close so the deletes reach the main file, mirroring the state
// prepareExportDB hands to vacuumDatabase.
err = db.Close()
if err != nil {
t.Fatalf("failed to close database: %v", err)
}
beforeInfo, err := fs.Stat(dbPath)
if err != nil {
t.Fatalf("failed to stat database before vacuum: %v", err)
}
beforeBytes, err := afero.ReadFile(fs, dbPath)
if err != nil {
t.Fatalf("failed to read database before vacuum: %v", err)
}
if !bytes.Contains(beforeBytes, marker) {
t.Fatalf("expected deleted-row data to linger before vacuum")
}
sm := &SnapshotManager{fs: fs}
err = sm.vacuumDatabase(ctx, dbPath)
if err != nil {
t.Fatalf("vacuumDatabase failed: %v", err)
}
afterBytes, err := afero.ReadFile(fs, dbPath)
if err != nil {
t.Fatalf("failed to read database after vacuum: %v", err)
}
if bytes.Contains(afterBytes, marker) {
t.Fatalf("deleted-row data survived vacuum in the uploaded file")
}
afterInfo, err := fs.Stat(dbPath)
if err != nil {
t.Fatalf("failed to stat database after vacuum: %v", err)
}
if afterInfo.Size() >= beforeInfo.Size() {
t.Fatalf("expected vacuum to shrink the file: before=%d after=%d",
beforeInfo.Size(), afterInfo.Size())
}
}
func TestCleanSnapshotDBEmptySnapshot(t *testing.T) { func TestCleanSnapshotDBEmptySnapshot(t *testing.T) {
// Initialize logger // Initialize logger
log.Initialize(log.Config{}) log.Initialize(log.Config{})
+54 -79
View File
@@ -46,18 +46,31 @@ func (f *FileStorer) SetFilesystem(fs afero.Fs) {
// storage base path. // storage base path.
const storageDirPerm = 0o755 const storageDirPerm = 0o755
// tempSuffix marks a partially written object. writeAtomic streams into a
// temp file carrying this suffix and only renames it onto the real key once
// the whole object is on disk, so an interrupted write can never leave a
// truncated object at the key a later run would Stat and trust as a complete
// blob. List and ListStream skip these files, so a leftover from an
// interrupted write is never listed or trusted as a blob; it is otherwise
// harmless and is overwritten when the same key is written again.
const tempSuffix = ".partial"
// Put stores data at the specified key. // Put stores data at the specified key.
func (f *FileStorer) Put(_ context.Context, key string, data io.Reader) error { func (f *FileStorer) Put(_ context.Context, key string, data io.Reader) error {
return f.writeAtomic(key, data, nil) path := f.fullPath(key)
// Create parent directories
dir := filepath.Dir(path)
err := f.fs.MkdirAll(dir, storageDirPerm)
if err != nil {
return fmt.Errorf("creating directories: %w", err)
}
file, err := f.fs.Create(path)
if err != nil {
return fmt.Errorf("creating file: %w", err)
}
defer func() { _ = file.Close() }()
_, err = io.Copy(file, data)
if err != nil {
return fmt.Errorf("writing file: %w", err)
}
return nil
} }
// PutWithProgress stores data with progress reporting. // PutWithProgress stores data with progress reporting.
@@ -65,7 +78,35 @@ func (f *FileStorer) PutWithProgress(
_ context.Context, key string, data io.Reader, _ context.Context, key string, data io.Reader,
_ int64, progress ProgressCallback, _ int64, progress ProgressCallback,
) error { ) error {
return f.writeAtomic(key, data, progress) path := f.fullPath(key)
// Create parent directories
dir := filepath.Dir(path)
err := f.fs.MkdirAll(dir, storageDirPerm)
if err != nil {
return fmt.Errorf("creating directories: %w", err)
}
file, err := f.fs.Create(path)
if err != nil {
return fmt.Errorf("creating file: %w", err)
}
defer func() { _ = file.Close() }()
// Wrap with progress tracking
pw := &progressWriter{
writer: file,
callback: progress,
}
_, err = io.Copy(pw, data)
if err != nil {
return fmt.Errorf("writing file: %w", err)
}
return nil
} }
// Get retrieves data from the specified key. // Get retrieves data from the specified key.
@@ -147,7 +188,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
default: default:
} }
if !info.IsDir() && !strings.HasSuffix(info.Name(), tempSuffix) { if !info.IsDir() {
// Convert back to key (relative path from basePath) // Convert back to key (relative path from basePath)
relPath, err := filepath.Rel(f.basePath, path) relPath, err := filepath.Rel(f.basePath, path)
if err != nil { if err != nil {
@@ -204,7 +245,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
return nil //nolint:nilerr // continue walking despite errors return nil //nolint:nilerr // continue walking despite errors
} }
if !info.IsDir() && !strings.HasSuffix(info.Name(), tempSuffix) { if !info.IsDir() {
relPath, err := filepath.Rel(f.basePath, path) relPath, err := filepath.Rel(f.basePath, path)
if err != nil { if err != nil {
ch <- ObjectInfo{Err: fmt.Errorf("computing relative path: %w", err)} ch <- ObjectInfo{Err: fmt.Errorf("computing relative path: %w", err)}
@@ -234,72 +275,6 @@ func (f *FileStorer) Info() Info {
} }
} }
// writeAtomic streams data into a temp file in the destination directory,
// fsyncs it, and renames it onto the final key. The key therefore appears
// only once the whole object has been durably written; a failure part-way
// leaves a temp file (removed here on the failing path) rather than a
// truncated object at the key.
func (f *FileStorer) writeAtomic(
key string, data io.Reader, progress ProgressCallback,
) error {
path := f.fullPath(key)
dir := filepath.Dir(path)
err := f.fs.MkdirAll(dir, storageDirPerm)
if err != nil {
return fmt.Errorf("creating directories: %w", err)
}
tmp, err := afero.TempFile(f.fs, dir, filepath.Base(path)+"-*"+tempSuffix)
if err != nil {
return fmt.Errorf("creating temp file: %w", err)
}
tmpPath := tmp.Name()
// Remove the temp file unless the rename below claims it. On the success
// path renamed is true, so the deferred Close and Remove are harmless
// no-ops on a name that no longer exists.
renamed := false
defer func() {
_ = tmp.Close()
if !renamed {
_ = f.fs.Remove(tmpPath)
}
}()
var w io.Writer = tmp
if progress != nil {
w = &progressWriter{writer: tmp, callback: progress}
}
_, err = io.Copy(w, data)
if err != nil {
return fmt.Errorf("writing file: %w", err)
}
err = tmp.Sync()
if err != nil {
return fmt.Errorf("syncing temp file: %w", err)
}
err = tmp.Close()
if err != nil {
return fmt.Errorf("closing temp file: %w", err)
}
err = f.fs.Rename(tmpPath, path)
if err != nil {
return fmt.Errorf("renaming temp file: %w", err)
}
renamed = true
return nil
}
// fullPath returns the full filesystem path for a key. // fullPath returns the full filesystem path for a key.
func (f *FileStorer) fullPath(key string) string { func (f *FileStorer) fullPath(key string) string {
return filepath.Join(f.basePath, key) return filepath.Join(f.basePath, key)
-119
View File
@@ -1,119 +0,0 @@
package storage_test
import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"sneak.berlin/go/vaultik/internal/storage"
)
// errStreamInterrupted stands in for an upload cut off mid-stream.
var errStreamInterrupted = errors.New("connection reset mid-upload")
// failingReader yields its data once, then fails.
type failingReader struct {
data []byte
done bool
}
func (r *failingReader) Read(p []byte) (int, error) {
if r.done {
return 0, errStreamInterrupted
}
n := copy(p, r.data)
r.done = true
return n, nil
}
// TestFileStorer_InterruptedWriteLeavesNoTrustedObject checks that a write
// cut off mid-stream leaves nothing at the destination key, so a later run
// cannot Stat a truncated object and trust it as a complete blob.
func TestFileStorer_InterruptedWriteLeavesNoTrustedObject(t *testing.T) {
t.Parallel()
f, err := storage.NewFileStorer(t.TempDir())
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
ctx := context.Background()
key := "blobs/aa/bb/aabbccddeeff"
err = f.PutWithProgress(ctx, key, &failingReader{data: []byte("partial")}, 4096, nil)
if err == nil {
t.Fatal("expected the interrupted write to fail, got nil")
}
_, err = f.Stat(ctx, key)
if !errors.Is(err, storage.ErrNotFound) {
t.Fatalf("expected key absent after interrupted write, got Stat err %v", err)
}
keys, err := f.List(ctx, "blobs/")
if err != nil {
t.Fatalf("List: %v", err)
}
if len(keys) != 0 {
t.Fatalf("expected no keys listed after interrupted write, got %v", keys)
}
}
// TestFileStorer_ListSkipsPartialFiles checks that a leftover temp file (the
// storage layer names them with a ".partial" suffix) is never surfaced as a
// key by List or ListStream.
func TestFileStorer_ListSkipsPartialFiles(t *testing.T) {
t.Parallel()
base := t.TempDir()
f, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
ctx := context.Background()
realKey := "blobs/aa/bb/aabbccddeeff"
err = f.Put(ctx, realKey, strings.NewReader("blob-bytes"))
if err != nil {
t.Fatalf("Put: %v", err)
}
// A stray temp file, as an interrupted write would leave behind.
leftover := filepath.Join(base, "blobs/aa/bb/aabbccddeeff-123456.partial")
err = os.WriteFile(leftover, []byte("half"), 0o600)
if err != nil {
t.Fatalf("writing leftover temp file: %v", err)
}
keys, err := f.List(ctx, "blobs/")
if err != nil {
t.Fatalf("List: %v", err)
}
if len(keys) != 1 || keys[0] != realKey {
t.Fatalf("List should return only the real key, got %v", keys)
}
var streamed []string
for obj := range f.ListStream(ctx, "blobs/") {
if obj.Err != nil {
t.Fatalf("ListStream: %v", obj.Err)
}
streamed = append(streamed, obj.Key)
}
if len(streamed) != 1 || streamed[0] != realKey {
t.Fatalf("ListStream should return only the real key, got %v", streamed)
}
}
+1 -16
View File
@@ -38,29 +38,14 @@ func (s *S3Storer) PutWithProgress(
} }
// Get retrieves data from the specified key. // Get retrieves data from the specified key.
// Returns ErrNotFound if the object does not exist.
func (s *S3Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) { func (s *S3Storer) Get(ctx context.Context, key string) (io.ReadCloser, error) {
rc, err := s.client.GetObject(ctx, key) return s.client.GetObject(ctx, key)
if err != nil {
if s3.IsNotFound(err) {
return nil, fmt.Errorf("get %q: %w", key, ErrNotFound)
}
return nil, err
}
return rc, nil
} }
// Stat returns metadata about an object without retrieving its contents. // Stat returns metadata about an object without retrieving its contents.
// Returns ErrNotFound if the object does not exist.
func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) { func (s *S3Storer) Stat(ctx context.Context, key string) (*ObjectInfo, error) {
info, err := s.client.StatObject(ctx, key) info, err := s.client.StatObject(ctx, key)
if err != nil { if err != nil {
if s3.IsNotFound(err) {
return nil, fmt.Errorf("stat %q: %w", key, ErrNotFound)
}
return nil, err return nil, err
} }
-59
View File
@@ -1,59 +0,0 @@
package storage_test
import (
"context"
"errors"
"net/http/httptest"
"testing"
"github.com/johannesboyne/gofakes3"
"github.com/johannesboyne/gofakes3/backend/s3mem"
"sneak.berlin/go/vaultik/internal/s3"
"sneak.berlin/go/vaultik/internal/storage"
)
// TestS3StorerMissingKeyMapsToErrNotFound verifies that the s3 backend reports
// a missing object as storage.ErrNotFound, matching the file and rclone
// backends and the Storer contract. Without the mapping, Get and Stat leak the
// raw SDK error and errors.Is(err, storage.ErrNotFound) is false.
//
//nolint:paralleltest // shares an in-process S3 server via t.Cleanup
func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
const bucket = "test-bucket"
backend := s3mem.New()
err := backend.CreateBucket(bucket)
if err != nil {
t.Fatalf("create bucket: %v", err)
}
srv := httptest.NewServer(gofakes3.New(backend).Server())
t.Cleanup(srv.Close)
ctx := context.Background()
client, err := s3.NewClient(ctx, s3.Config{
Endpoint: srv.URL,
Bucket: bucket,
AccessKeyID: "test",
SecretAccessKey: "test",
Region: "us-east-1",
})
if err != nil {
t.Fatalf("new client: %v", err)
}
storer := storage.NewS3Storer(client)
_, err = storer.Get(ctx, "does-not-exist")
if !errors.Is(err, storage.ErrNotFound) {
t.Errorf("Get on missing key: got %v, want ErrNotFound", err)
}
_, err = storer.Stat(ctx, "does-not-exist")
if !errors.Is(err, storage.ErrNotFound) {
t.Errorf("Stat on missing key: got %v, want ErrNotFound", err)
}
}
-108
View File
@@ -1,108 +0,0 @@
package vaultik_test
import (
"context"
"io"
"os"
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob backs up a real
// snapshot with the on-disk storage backend, runs deep verification on
// it, then flips a byte inside one stored blob and runs deep
// verification again. A healthy snapshot must pass; a corrupted blob
// must fail. The healthy case is the regression guard: deep
// verification used to hash the encrypted blob bytes and compare them
// to the blob's ID (the double SHA256 of the plaintext), so it reported
// every healthy blob as corrupt.
func TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "source")
storeDir := filepath.Join(tempDir, "remote")
dbPath := filepath.Join(tempDir, "index.sqlite")
chunkSize := int64(64 * 1024)
maxBlobSize := int64(512 * 1024)
// One file large enough to span several chunks within a single blob.
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
require.NoError(t, afero.WriteFile(fs,
filepath.Join(dataDir, "data.bin"),
bytesPattern("deep-", int(chunkSize*3)), 0o644))
ctx := context.Background()
// runFileStorageBackup writes a real snapshot to storeDir and closes
// the source index, so verification runs from remote bytes only.
cfg, storer, snapshotID := runFileStorageBackup(
ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
newVerifier := func() *vaultik.Vaultik {
v := &vaultik.Vaultik{
Config: cfg,
Storage: storer,
Fs: fs,
Stdout: io.Discard,
Stderr: io.Discard,
UI: ui.NewWithColor(io.Discard, false),
}
v.SetContext(ctx)
return v
}
require.NoError(t,
newVerifier().RunDeepVerify(snapshotID, &vaultik.VerifyOptions{Deep: true}),
"deep verify should pass on a healthy snapshot")
// Flip a byte inside one blob without changing its length, so the
// blob-existence and size checks still pass and verification reaches
// the blob-content stage.
corruptOneBlob(t, fs, filepath.Join(storeDir, "blobs"))
require.Error(t,
newVerifier().RunDeepVerify(snapshotID, &vaultik.VerifyOptions{Deep: true}),
"deep verify should fail on a corrupted blob")
}
// corruptOneBlob flips a middle byte of the first blob file found under
// blobsDir, leaving the file length unchanged.
func corruptOneBlob(t *testing.T, fs afero.Fs, blobsDir string) {
t.Helper()
var blobPath string
err := afero.Walk(fs, blobsDir,
func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if blobPath == "" && !info.IsDir() {
blobPath = path
}
return nil
})
require.NoError(t, err)
require.NotEmpty(t, blobPath, "expected at least one blob on disk")
data, err := afero.ReadFile(fs, blobPath)
require.NoError(t, err)
require.NotEmpty(t, data)
data[len(data)/2] ^= 0xff
require.NoError(t, afero.WriteFile(fs, blobPath, data, 0o644))
}
+3 -9
View File
@@ -33,9 +33,8 @@ func ubytes(n int64) string {
var ( var (
errMalformedSnapshotID = errors.New( errMalformedSnapshotID = errors.New(
"invalid snapshot ID format: expected hostname_snapshotname_timestamp") "invalid snapshot ID format: expected hostname_snapshotname_timestamp")
errInvalidDuration = errors.New("invalid duration") errInvalidDuration = errors.New("invalid duration")
errUnknownTimeUnit = errors.New("unknown time unit") errUnknownTimeUnit = errors.New("unknown time unit")
errNegativeDuration = errors.New("negative durations are not supported")
) )
// Time-unit lengths used by parseDuration. // Time-unit lengths used by parseDuration.
@@ -139,13 +138,8 @@ func parseSnapshotName(snapshotID string) string {
// parseDuration parses a duration string with support for human-friendly units: // parseDuration parses a duration string with support for human-friendly units:
// d/day/days, w/week/weeks, mo/month/months, y/year/years, plus standard Go // d/day/days, w/week/weeks, mo/month/months, y/year/years, plus standard Go
// duration units. Following Go, m is minutes and mo is months. A bare number, // duration units (h, m, s).
// an unknown unit, and a negative value are all rejected.
func parseDuration(s string) (time.Duration, error) { func parseDuration(s string) (time.Duration, error) {
if strings.HasPrefix(strings.TrimSpace(s), "-") {
return 0, errNegativeDuration
}
d, err := time.ParseDuration(s) d, err := time.ParseDuration(s)
if err == nil { if err == nil {
return d, nil return d, nil
+3 -22
View File
@@ -51,32 +51,13 @@ func TestParseDuration(t *testing.T) {
want time.Duration want time.Duration
err bool err bool
}{ }{
// Go units, including the m-is-minutes / mo-is-months distinction
// that this parser exists to keep straight.
{"10ns", 10 * time.Nanosecond, false},
{"10us", 10 * time.Microsecond, false},
{"500ms", 500 * time.Millisecond, false},
{"30s", 30 * time.Second, false},
{"6m", 6 * time.Minute, false},
{"1h", time.Hour, false},
// Extended calendar units.
{"30d", 30 * 24 * time.Hour, false}, {"30d", 30 * 24 * time.Hour, false},
{"3days", 3 * 24 * time.Hour, false},
{"4w", 4 * 7 * 24 * time.Hour, false}, {"4w", 4 * 7 * 24 * time.Hour, false},
{"2weeks", 2 * 7 * 24 * time.Hour, false}, {"6mo", 6 * 30 * 24 * time.Hour, false},
{"6mo", 180 * 24 * time.Hour, false},
{"1month", 30 * 24 * time.Hour, false},
{"1y", 365 * 24 * time.Hour, false}, {"1y", 365 * 24 * time.Hour, false},
{"2years", 2 * 365 * 24 * time.Hour, false},
// Combined units.
{"2w3d", 2*7*24*time.Hour + 3*24*time.Hour, false}, {"2w3d", 2*7*24*time.Hour + 3*24*time.Hour, false},
{"1y6mo", 365*24*time.Hour + 180*24*time.Hour, false}, {"1h", time.Hour, false},
// Rejected inputs. {"30s", 30 * time.Second, false},
{"6", 0, true}, // bare number, no unit
{"5x", 0, true}, // unknown unit
{"-5d", 0, true}, // negative, extended unit
{"-5h", 0, true}, // negative, Go unit
{"", 0, true}, // empty
{"garbage", 0, true}, {"garbage", 0, true},
} }
-79
View File
@@ -1,79 +0,0 @@
package vaultik //nolint:testpackage // exercises unexported count helpers
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
)
// TestTableCountForReportSurfacesReadFailure is the regression guard for
// the discarded-error bug: getTableCount for a table its query cannot
// resolve must not silently become 0. A count that could not be read is
// reported as unknown, which a reader can tell apart from an empty table.
//
//nolint:paralleltest // installs the global logger via log.Initialize
func TestTableCountForReportSurfacesReadFailure(t *testing.T) {
log.Initialize(log.Config{})
ctx := context.Background()
db, err := database.New(ctx, ":memory:")
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
v := &Vaultik{DB: db}
v.SetContext(ctx)
// A table present in the schema reads as a real count.
blobs := v.tableCountForReport("blobs")
require.NotNil(t, blobs, "an existing table must read as a real count")
assert.Equal(t, int64(0), *blobs)
// A syntactically valid name the sanitizer accepts but whose table
// the query cannot resolve is the exact shape #96 describes: a
// would-be loud failure that used to be discarded into a 0.
_, err = v.getTableCount("snapshots_missing")
require.Error(t, err, "a query against a nonexistent table must fail")
missing := v.tableCountForReport("snapshots_missing")
assert.Nil(t, missing, "a failed read is unknown, not a count")
// The rendered count for a failed read must say unknown, never 0.
assert.Equal(t, countUnknown, countText(missing))
assert.NotEqual(t, "0", countText(missing))
}
// TestCountTextDistinguishesEmptyFromUnknown pins the distinction the
// output has to preserve: 0 means the table was empty, "unknown" means
// the count could not be read.
func TestCountTextDistinguishesEmptyFromUnknown(t *testing.T) {
t.Parallel()
zero := int64(0)
seven := int64(7)
assert.Equal(t, "0", countText(&zero))
assert.Equal(t, "7", countText(&seven))
assert.Equal(t, countUnknown, countText(nil))
}
// TestCountDiffUnknownWhenEitherSideUnknown checks that a delta computed
// from an unreadable count is itself unknown rather than a plausible
// number.
func TestCountDiffUnknownWhenEitherSideUnknown(t *testing.T) {
t.Parallel()
before := int64(10)
after := int64(3)
require.NotNil(t, countDiff(&before, &after))
assert.Equal(t, int64(7), *countDiff(&before, &after))
assert.Nil(t, countDiff(nil, &after), "unknown before yields unknown delta")
assert.Nil(t, countDiff(&before, nil), "unknown after yields unknown delta")
assert.Nil(t, countDiff(nil, nil))
}
+26 -79
View File
@@ -8,7 +8,6 @@ import (
"path/filepath" "path/filepath"
"regexp" "regexp"
"sort" "sort"
"strconv"
"strings" "strings"
"time" "time"
@@ -1541,17 +1540,12 @@ func (v *Vaultik) outputRemoveJSON(result *RemoveResult) error {
return encoder.Encode(result) return encoder.Encode(result)
} }
// PruneResult contains statistics about the prune operation. // PruneResult contains statistics about the prune operation
// SnapshotsDeleted counts snapshots actually deleted. FilesDeleted,
// ChunksDeleted, and BlobsDeleted are derived from before/after row
// counts of the local index; each is nil when a count could not be read,
// so an unreadable count is reported as unknown rather than silently
// as 0.
type PruneResult struct { type PruneResult struct {
SnapshotsDeleted int64 SnapshotsDeleted int64
FilesDeleted *int64 FilesDeleted int64
ChunksDeleted *int64 ChunksDeleted int64
BlobsDeleted *int64 BlobsDeleted int64
} }
// PruneDatabase removes incomplete snapshots and orphaned files, chunks, // PruneDatabase removes incomplete snapshots and orphaned files, chunks,
@@ -1566,7 +1560,7 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
result := &PruneResult{} result := &PruneResult{}
// Snapshot counts before deletion of incompletes. // Snapshot counts before deletion of incompletes.
snapshotCountBefore := v.tableCountForReport("snapshots") snapshotCountBefore, _ := v.getTableCount("snapshots")
// First, delete any incomplete snapshots // First, delete any incomplete snapshots
incompleteSnapshots, err := v.Repositories.Snapshots.GetIncompleteSnapshots(v.ctx) incompleteSnapshots, err := v.Repositories.Snapshots.GetIncompleteSnapshots(v.ctx)
@@ -1581,9 +1575,9 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
} }
// Get counts before cleanup for reporting // Get counts before cleanup for reporting
fileCountBefore := v.tableCountForReport("files") fileCountBefore, _ := v.getTableCount("files")
chunkCountBefore := v.tableCountForReport("chunks") chunkCountBefore, _ := v.getTableCount("chunks")
blobCountBefore := v.tableCountForReport("blobs") blobCountBefore, _ := v.getTableCount("blobs")
// Run the cleanup // Run the cleanup
err = v.SnapshotManager.CleanupOrphanedData(v.ctx) err = v.SnapshotManager.CleanupOrphanedData(v.ctx)
@@ -1592,83 +1586,36 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
} }
// Get counts after cleanup // Get counts after cleanup
fileCountAfter := v.tableCountForReport("files") fileCountAfter, _ := v.getTableCount("files")
chunkCountAfter := v.tableCountForReport("chunks") chunkCountAfter, _ := v.getTableCount("chunks")
blobCountAfter := v.tableCountForReport("blobs") blobCountAfter, _ := v.getTableCount("blobs")
result.FilesDeleted = countDiff(fileCountBefore, fileCountAfter) result.FilesDeleted = fileCountBefore - fileCountAfter
result.ChunksDeleted = countDiff(chunkCountBefore, chunkCountAfter) result.ChunksDeleted = chunkCountBefore - chunkCountAfter
result.BlobsDeleted = countDiff(blobCountBefore, blobCountAfter) result.BlobsDeleted = blobCountBefore - blobCountAfter
log.Info("Local database prune complete", log.Info("Local database prune complete",
"incomplete_snapshots", result.SnapshotsDeleted, "incomplete_snapshots", result.SnapshotsDeleted,
"orphaned_files", countText(result.FilesDeleted), "orphaned_files", result.FilesDeleted,
"orphaned_chunks", countText(result.ChunksDeleted), "orphaned_chunks", result.ChunksDeleted,
"orphaned_blobs", countText(result.BlobsDeleted), "orphaned_blobs", result.BlobsDeleted,
) )
// Snapshots remaining after removing the incomplete ones; unknown if snapshotCountAfter := snapshotCountBefore - result.SnapshotsDeleted
// the pre-prune snapshot count could not be read.
snapshotsRemain := countDiff(snapshotCountBefore, &result.SnapshotsDeleted)
v.UI.Completef("Pruned local index database.") v.UI.Completef("Pruned local index database.")
v.UI.Detailf("Incomplete snapshots: %s removed (%s remain).", v.UI.Detailf("Incomplete snapshots: %d removed (%d remain).",
countText(&result.SnapshotsDeleted), countText(snapshotsRemain)) result.SnapshotsDeleted, snapshotCountAfter)
v.UI.Detailf("Orphaned files: %s removed (%s remain).", v.UI.Detailf("Orphaned files: %d removed (%d remain).",
countText(result.FilesDeleted), countText(fileCountAfter)) result.FilesDeleted, fileCountAfter)
v.UI.Detailf("Orphaned chunks: %s removed (%s remain).", v.UI.Detailf("Orphaned chunks: %d removed (%d remain).",
countText(result.ChunksDeleted), countText(chunkCountAfter)) result.ChunksDeleted, chunkCountAfter)
v.UI.Detailf("Orphaned blobs: %s removed (%s remain).", v.UI.Detailf("Orphaned blobs: %d removed (%d remain).",
countText(result.BlobsDeleted), countText(blobCountAfter)) result.BlobsDeleted, blobCountAfter)
return result, nil return result, nil
} }
// countUnknown is what a count reads as when its query could not be run,
// distinct from "0", which means the table really was empty.
const countUnknown = "unknown"
// tableCountForReport returns the row count of a table for the prune
// summary, or nil if the count could not be read. A read failure is
// logged at warn — visible even under --json, which routes warnings to
// stderr — and then rendered as unknown rather than silently becoming 0,
// so a broken query is a visible failure instead of a plausible wrong
// number.
func (v *Vaultik) tableCountForReport(tableName string) *int64 {
count, err := v.getTableCount(tableName)
if err != nil {
log.Warn("could not read table row count for prune summary",
"table", tableName, "error", err)
return nil
}
return &count
}
// countDiff returns before-after, or nil if either count is unknown so
// that an unreadable count does not collapse into a plausible delta.
func countDiff(before, after *int64) *int64 {
if before == nil || after == nil {
return nil
}
diff := *before - *after
return &diff
}
// countText renders a count that may be unknown: nil (the read failed)
// becomes "unknown", never "0", so a reader can tell an empty table from
// one that could not be queried.
func countText(count *int64) string {
if count == nil {
return countUnknown
}
return strconv.FormatInt(*count, 10)
}
// validTableNameRe matches table names containing only lowercase // validTableNameRe matches table names containing only lowercase
// alphanumeric characters and underscores. // alphanumeric characters and underscores.
var validTableNameRe = regexp.MustCompile(`^[a-z0-9_]+$`) var validTableNameRe = regexp.MustCompile(`^[a-z0-9_]+$`)
+14 -19
View File
@@ -344,8 +344,12 @@ func (v *Vaultik) verifyBlob(blobInfo snapshot.BlobInfo, db *sql.DB) error {
return fmt.Errorf("failed to get decryptor: %w", err) return fmt.Errorf("failed to get decryptor: %w", err)
} }
// Decrypt blob // Hash the encrypted blob data as it streams through to decryption
decryptedReader, err := decryptor.DecryptStream(reader) blobHasher := sha256.New()
teeReader := io.TeeReader(reader, blobHasher)
// Decrypt blob (reading through teeReader to hash encrypted data)
decryptedReader, err := decryptor.DecryptStream(teeReader)
if err != nil { if err != nil {
return fmt.Errorf("failed to decrypt: %w", err) return fmt.Errorf("failed to decrypt: %w", err)
} }
@@ -357,19 +361,12 @@ func (v *Vaultik) verifyBlob(blobInfo snapshot.BlobInfo, db *sql.DB) error {
} }
defer decompressor.Close() defer decompressor.Close()
// A blob's hash — its remote name — is the double SHA256 of its chunkCount, err := v.verifyBlobChunks(db, blobInfo.Hash, decompressor)
// decompressed plaintext (see blobgen.Writer.Sum256), not of the
// encrypted bytes. Hash the plaintext as chunk verification streams
// it, then compare on completion.
plaintextHasher := sha256.New()
hashedStream := io.TeeReader(decompressor, plaintextHasher)
chunkCount, err := v.verifyBlobChunks(db, blobInfo.Hash, hashedStream)
if err != nil { if err != nil {
return err return err
} }
err = v.verifyBlobFinalIntegrity(hashedStream, plaintextHasher, blobInfo.Hash) err = v.verifyBlobFinalIntegrity(decompressor, blobHasher, blobInfo.Hash)
if err != nil { if err != nil {
return err return err
} }
@@ -473,13 +470,14 @@ func (v *Vaultik) verifyBlobChunks(
} }
// verifyBlobFinalIntegrity checks that no trailing data exists in the // verifyBlobFinalIntegrity checks that no trailing data exists in the
// decompressed stream and that the blob hash matches the expected value. // decompressed stream and that the encrypted blob hash matches the
// expected value.
func (v *Vaultik) verifyBlobFinalIntegrity( func (v *Vaultik) verifyBlobFinalIntegrity(
plaintext io.Reader, plaintextHasher hash.Hash, expectedHash string, decompressor io.Reader, blobHasher hash.Hash, expectedHash string,
) error { ) error {
// Verify no remaining data in blob - if the chunk list is accurate, // Verify no remaining data in blob - if the chunk list is accurate,
// the blob should be fully consumed. // the blob should be fully consumed.
remaining, err := io.Copy(io.Discard, plaintext) remaining, err := io.Copy(io.Discard, decompressor)
if err != nil { if err != nil {
return fmt.Errorf("failed to check for remaining blob data: %w", err) return fmt.Errorf("failed to check for remaining blob data: %w", err)
} }
@@ -488,11 +486,8 @@ func (v *Vaultik) verifyBlobFinalIntegrity(
return fmt.Errorf("%w: %d bytes", errTrailingBlobData, remaining) return fmt.Errorf("%w: %d bytes", errTrailingBlobData, remaining)
} }
// The blob hash is the double SHA256 of its plaintext content. // Verify blob hash matches the encrypted data we downloaded
firstHash := plaintextHasher.Sum(nil) calculatedBlobHash := hex.EncodeToString(blobHasher.Sum(nil))
secondHash := sha256.Sum256(firstHash)
calculatedBlobHash := hex.EncodeToString(secondHash[:])
if calculatedBlobHash != expectedHash { if calculatedBlobHash != expectedHash {
return fmt.Errorf("%w: calculated %s, expected %s", return fmt.Errorf("%w: calculated %s, expected %s",
errBlobHashMismatch, calculatedBlobHash, expectedHash) errBlobHashMismatch, calculatedBlobHash, expectedHash)
+3
View File
@@ -114,6 +114,9 @@ main() {
# from CI. Nothing on the host is ever used as a linter, at any # from CI. Nothing on the host is ever used as a linter, at any
# version, so installing one here would buy nothing. # version, so installing one here would buy nothing.
# sqlite3 CLI: the test suite shells out to it (VACUUM).
if missing sqlite3; then pkg_install sqlite sqlite3 sqlite sqlite; fi
# goreleaser, at the version pinned by script/install-goreleaser and # goreleaser, at the version pinned by script/install-goreleaser and
# verified against a hardcoded sha256. Package managers are not used # verified against a hardcoded sha256. Package managers are not used
# for it: they ship whatever version they happen to carry, and the # for it: they ship whatever version they happen to carry, and the
-161
View File
@@ -1,161 +0,0 @@
#!/bin/sh
# script/install-go: install the Go toolchain pinned by go.mod into the
# repo-local tool directory, verified against a committed sha256. Our
# own extension to scripts-to-rule-them-all. Idempotent: exits at once
# when the pinned toolchain is already installed.
#
# Only .gitea/workflows/release.yml calls this. goreleaser is not a
# compiler: it shells out to `go` for the `before:` hook and for every
# one of the four cross-compiles, so the release runner needs a Go
# toolchain on PATH. check.yml never does -- it builds inside the
# digest-pinned Dockerfile images -- so this is the release path's only
# host Go, and per REPO_POLICIES.md it must be pinned by hash.
# actions/setup-go exposes no checksum input, so Go is installed the way
# script/install-goreleaser installs goreleaser: download the exact
# archive from go.dev and refuse it unless its sha256 matches the value
# committed below.
#
# The version is go.mod's `go` directive, the single source of truth for
# the toolchain. GO_VERSION below MUST equal it, and this script fails
# when they disagree -- so bumping Go is one reviewed change touching
# go.mod, the checksum here, and the Dockerfile golang digest together.
#
# Linux only, because that is what the release runner is. A darwin dev
# building a snapshot uses their own Go; supporting an OS means adding
# its checksums.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Go 1.26.1, 2026-09-21. Checksums are the sha256 values go.dev publishes
# for each archive at https://go.dev/dl/ (also in its ?mode=json
# manifest).
GO_VERSION="1.26.1"
SHA256_LINUX_AMD64="031f088e5d955bab8657ede27ad4e3bc5b7c1ba281f05f245bcc304f327c987a"
SHA256_LINUX_ARM64="a290581cfe4fe28ddd737dde3095f3dbeb7f2e4065cab4eae44dfc53b760c2f7"
GOROOT_DIR="$ROOT/.tool/go"
GOCMD="$GOROOT_DIR/bin/go"
# The `go` directive in go.mod, e.g. "1.26.1" from `go 1.26.1`.
gomod_go_version() {
sed -n 's/^go \([0-9][0-9.]*\).*/\1/p' "$ROOT/go.mod" | head -n 1
}
# Print the version of the go at $1 as "1.26.1", or nothing if it is not
# usable. `go version` prints "go version go1.26.1 linux/amd64".
go_version() {
[ -x "$1" ] || return 0
"$1" version 2>/dev/null |
sed -n 's/^go version go\([0-9][0-9.]*\) .*/\1/p' |
head -n 1
}
verify_sha256() {
file="$1"
want="$2"
if command -v sha256sum >/dev/null 2>&1; then
got="$(sha256sum "$file" | cut -d' ' -f1)"
elif command -v shasum >/dev/null 2>&1; then
got="$(shasum -a 256 "$file" | cut -d' ' -f1)"
else
echo "install-go: no sha256sum or shasum available" >&2
return 1
fi
if [ "$got" != "$want" ]; then
echo "install-go: checksum mismatch for $file" >&2
echo " expected: $want" >&2
echo " actual: $got" >&2
return 1
fi
}
# On a Gitea/GitHub Actions runner, put the toolchain on PATH for the
# steps that follow by appending to the file named by $GITHUB_PATH. A
# no-op off CI, where the caller manages its own PATH.
export_ci_path() {
[ -n "${GITHUB_PATH:-}" ] || return 0
echo "$GOROOT_DIR/bin" >>"$GITHUB_PATH"
}
main() {
cd "$ROOT"
want="$(gomod_go_version)"
if [ "$want" != "$GO_VERSION" ]; then
echo "install-go: go.mod says go $want but this script pins" \
"$GO_VERSION." >&2
echo " Update GO_VERSION and the checksums in this script to" \
"match go.mod." >&2
exit 1
fi
# Already installed from a previous run? Then just fix PATH and stop.
if [ "$(go_version "$GOCMD")" = "$GO_VERSION" ]; then
echo "go $GO_VERSION already installed in .tool/go"
export_ci_path
return 0
fi
os="$(uname -s)"
arch="$(uname -m)"
case "$os" in
Linux) os="linux" ;;
*)
echo "install-go: unsupported OS $os (release runner is Linux)" >&2
exit 1
;;
esac
case "$arch" in
x86_64 | amd64)
arch="amd64"
sum="$SHA256_LINUX_AMD64"
;;
arm64 | aarch64)
arch="arm64"
sum="$SHA256_LINUX_ARM64"
;;
*)
echo "install-go: no pinned checksum for architecture $arch" >&2
exit 1
;;
esac
archive="go${GO_VERSION}.${os}-${arch}.tar.gz"
url="https://go.dev/dl/${archive}"
if ! command -v curl >/dev/null 2>&1; then
echo "install-go: curl is required" >&2
exit 1
fi
dl="$(mktemp -d)"
mkdir -p "$ROOT/.tool"
stage="$(mktemp -d "$ROOT/.tool/.go-install.XXXXXX")"
# shellcheck disable=SC2064 # expand the paths now, not at trap time
trap "rm -rf '$dl' '$stage'" EXIT INT TERM
echo "installing go $GO_VERSION for ${os}-${arch}"
curl -fsSL --retry 3 -o "$dl/$archive" "$url"
verify_sha256 "$dl/$archive" "$sum"
# The archive unpacks to a top-level `go/` directory. Extract it into
# a staging directory on the same filesystem as the destination, then
# rename it into place so a concurrent run never observes a
# half-written toolchain.
tar -xzf "$dl/$archive" -C "$stage"
rm -rf "$GOROOT_DIR"
mv "$stage/go" "$GOROOT_DIR"
installed="$(go_version "$GOCMD")"
if [ "$installed" != "$GO_VERSION" ]; then
echo "install-go: installed toolchain reports '$installed'," \
"expected '$GO_VERSION'" >&2
exit 1
fi
echo "go $GO_VERSION installed to .tool/go"
export_ci_path
}
main "$@"