1 Commits
Author SHA1 Message Date
sneak 2de79a0897 Report a prune count that could not be read as unknown, not 0 (closes #96)
check / check (pull_request) Failing after 0s
PruneDatabase read seven table counts with the error discarded, so a
query that could not run silently became 0 and the before/after delta
computed from it looked like real work.

Each read now goes through a helper that logs at warn on failure and
returns nil; nil renders as "unknown", never "0", so an empty table is
distinguishable from one that could not be queried. Deltas built from an
unknown count are themselves unknown. The failure is surfaced, not
propagated, so the command's failure conditions are unchanged. These
counts have no --json output — under --json the summary is suppressed
entirely — so nothing there can show a false 0.

Model: opus-4-8
2026-09-21 18:05:05 +00:00
5 changed files with 174 additions and 153 deletions
+5 -5
View File
@@ -72,11 +72,11 @@ RUN [ -n "$CHECK_EPOCH" ] || exit 1
# running, and exits 0 reporting `0 issues.` on a tree the real config # running, and exits 0 reporting `0 issues.` on a tree the real config
# fails. Demonstrated on this repo at this pin, recorded on # fails. Demonstrated on this repo at this pin, recorded on
# https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted # https://git.eeqj.de/sneak/vaultik/pulls/114: with a planted
# over-length line, `script/lint` exits 1 naming the `revive` finding # over-length line, `script/lint` exits 1 naming the `lll` finding with
# with `linters:` and exits 0 with `linterz:`. A set-but-ineffective # `linters:` and exits 0 with `linterz:`. A set-but-ineffective config
# config quietly falling back to defaults is precisely the false-green # quietly falling back to defaults is precisely the false-green class
# class this gate exists to eliminate, so it must not sit in the gate's # this gate exists to eliminate, so it must not sit in the gate's own
# own configuration. # configuration.
# #
# `config verify` catches it, and it does so OFFLINE at this pinned # `config verify` catches it, and it does so OFFLINE at this pinned
# version -- verified, not assumed. Under `docker run --network none` # version -- verified, not assumed. Under `docker run --network none`
+4 -14
View File
@@ -34,15 +34,6 @@ release" is exactly the contradiction
is distinguishable from one that could not be queried. The counts have is distinguishable from one that could not be queried. The counts have
no `--json` representation — under `--json` the summary is suppressed no `--json` representation — under `--json` the summary is suppressed
entirely — so nothing there can show a false `0`. entirely — so nothing there can show a false `0`.
- 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 - 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata
database through the `modernc.org/sqlite` driver instead of shelling database through the `modernc.org/sqlite` driver instead of shelling
@@ -97,11 +88,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
+151 -7
View File
@@ -28,11 +28,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 +37,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 +219,90 @@ 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())
for _, line := range shellCode(readRepoFile(t, name)) {
assertLinterIsContainerised(t, name, line)
}
}
}
// assertLinterIsContainerised fails if the line runs the linter without
// handing it to docker first. Position matters: docker has to come
// before the binary, or the line is running the host linter and merely
// mentioning docker afterwards.
func assertLinterIsContainerised(t *testing.T, name, line string) {
t.Helper()
at := strings.Index(line, linterBinary)
if at < 0 {
return
}
docker := strings.Index(line, "docker")
assert.True(t, docker >= 0 && docker < at,
"%s runs %s on the host; every lint run happens in a container"+
" (line: %s)", name, linterBinary, line)
}
// 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()
script := strings.Join([]string{
"#!/bin/sh",
"# a comment naming golangci-lint",
"cat >&2 <<EOF",
"prose naming golangci-lint, printed not executed",
"EOF",
"docker run --rm \\",
" \"$image\" \\",
" golangci-lint run ./...",
}, "\n")
assert.Equal(t,
[]string{"cat >&2 <<EOF", `docker run --rm "$image" golangci-lint run ./...`},
shellCode(script))
}
// 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 +407,70 @@ 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.
//
// 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.
func shellCode(contents string) []string {
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 = ""
}
return out
}
// heredocTerminator returns the terminator of the here-document a
// command opens, or "" if it opens none. Only the first on a line is
// recognised; nothing in script/ opens two.
func heredocTerminator(line string) string {
_, after, opens := strings.Cut(line, "<<")
if !opens {
return ""
}
// `<<-` strips leading tabs from the body; the terminator word is
// the same either way, and callers compare against trimmed lines.
word, _, _ := strings.Cut(strings.TrimPrefix(after, "-"), " ")
return strings.Trim(word, `'"`)
}
// 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 {
-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))
}
+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)