7e611b95dbe1221f198672dec752cb22d313b124
269
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 clawbot
|
7e611b95db |
Check blob sizes and the database in shallow verify (closes #169)
Shallow snapshot verify only checked that each blob object existed and then reported "All blobs verified", overstating what it did. It now compares each blob stored size against the manifest compressed_size, using the same comparison as the deep path, and checks that the snapshot encrypted database (db.zst.age) is present. A blob of the wrong size no longer counts as verified. The final line reports only what was checked: presence and size, not contents. The README verify description and the CLI short/long text are corrected to match. Removed the now-unused resolveAndDownloadManifest helper and errBlobsMissing sentinel. Model: opus-4-8 |
||
|
clawbot
|
4f27608560 |
Add negative and boundary tests for blobgen and types (closes #170)
Test-only. internal/blobgen and internal/types had no negative or boundary coverage. Adds, in package blobgen_test: Writer-to-Reader round trips at the 64 KiB age-segment edges for random and compressible data, checking plaintext, byte counts and the reader/writer hashes by decrypting; a wrong-identity open; truncation and single-byte corruption of a multi-segment blob at every region; trailing bytes, empty input and garbage; rejected and accepted compression levels; nil, empty and invalid recipients; and a failing destination. In package types_test: Value/Scan round trips, NULL, wrong-type and malformed Scan, Parse and IsZero for FileID and BlobID. The "cut right after the age header and nonce" truncation is excluded: it reads as valid and empty today and belongs to #152. Model: opus-4-8 |
||
|
clawbot
|
ae6aaaa388 |
Wait for the interrupted operation to clean up before exit (closes #159)
On SIGINT/SIGTERM the process could exit before the interrupted command cleanup defers ran, leaving decrypted data in the temp directory (the blob cache and the decrypted snapshot database). RunApp now mirrors fx run sequence: start, block on app.Wait(), then app.Stop(), returning only after Stop completes. fx delivers both an OS interrupt and the finished operation Shutdowner.Shutdown() on one channel. Stop runs the OnStop hooks; the operation hook cancels the command and waits for its goroutine to return (bounded by shutdownTimeout) before exit. The old code returned as soon as app.Done fired, without Stop, so a real interrupt unwound to os.Exit while cleanup still ran. Restore loops check the context between chunks and blobs so the wait ends promptly. A cli test drives RunApp through the OnStop hook. Model: opus-4-8 |
||
|
clawbot
|
f788668287 |
Remove the unused crypto path and write the blob-ID hash step once (closes #151)
Production encryption and decryption already run through blobgen; the crypto package (Encryptor, Decryptor, UpdateRecipients, the fx Module) and Vaultik.GetEncryptor/GetDecryptor had no production caller. Delete crypto and route verify --deep through the same blobgen reader restore uses, parsing the age key once. The second blob-ID hash step is now one exported blobgen.DoubleSHA256; Writer.Sum256 (the double hash) becomes Writer.ContentID so it no longer collides with Reader.Sum256 (the single plaintext hash). Also delete the never-adopted internal/types newtypes and the uncalled CleanupIncompleteSnapshots and its now-dead deleteSnapshot caller, and correct ARCHITECTURE.md. No production behavior changes. Model: opus-4-8 |
||
|
clawbot
|
238ce3985f |
Scrub example config of real credentials and internal hosts (closes #172)
config.example.yml carried a real-looking 20-char S3 access key id and 40-char secret, a private-address http:// endpoint, and a storage_url naming an internal rclone remote and pool path. Replace them with the same neutral placeholders the config init template uses: YOUR_ACCESS_KEY / YOUR_SECRET_KEY, a https://s3.example.com endpoint, a mybucket bucket, and rclone://myremote/path/to/backups. No behavior or other keys change. The credentials live in the commented-out s3 block, which the loader never parses, so the new test reads the file raw text to assert the placeholders are present and no http:// endpoint remains, and also loads it to confirm the active storage_url still parses. Model: opus-4-8 |
||
|
clawbot
|
548a7ae156 |
Give the local index and its export copy an explicit 0600 mode (closes #168)
The local index lists every backed-up path and chunk hash, but its file mode was left to the SQLite driver and the umask, so under a typical 022 umask a fresh index (and its -wal/-shm side files) landed world-readable. The snapshot export copied the index to snapshot.db with a permissive create as well. provideDatabase now calls ensureIndexFileMode before opening the driver: it creates the index 0600 if missing and chmods an existing one to 0600. Doing this before the driver opens the file matters because SQLite gives its -wal and -shm files the mode of the main database file. The export copy is now created 0600. Tests under umask 022 cover a fresh index, an existing 0644 index, and the export copy. Model: opus-4-8 |
||
|
clawbot
|
3a58377127 |
Parse age_recipients at config load and never echo the entry (closes #153)
Config.Validate now parses every age_recipients entry with age.ParseX25519Recipient, so a bad recipient fails at config load instead of deep in a backup after the snapshot row and tree walk. On failure the error names the position (age_recipients[N]) and never the value: a recipient string can itself be a secret key an operator pasted by mistake, and age's own error quotes its input. An entry starting with AGE-SECRET-KEY- gets a specific message. The remaining parse sites (blobgen.NewWriter, crypto NewEncryptor and UpdateRecipients), reachable by callers that skip config.Load, likewise drop the value and age's wrapped error, naming only the position. Model: opus-4-8 |
||
|
clawbot
|
a6434de57f |
Open the downloaded snapshot database read-only, on a private temp dir (closes #162)
Restore and deep verify used to open the decrypted snapshot database read-write through the local-index constructor, which applied migrations against whatever the file carried, and left the decrypted file in the shared temp directory. A forged file could redefine what restore queries return, and an interrupted open left decrypted metadata on disk. Add database.OpenReadOnly: opens the file read-only (mode=ro) with query_only and trusted_schema=OFF, never applies schema files, and refuses a file whose schema carries a trigger, view or virtual table or lacks an expected table. Restore and deep verify now both use it, each inside its own private (0700) temp directory removed on every return path. pickNextDownload returns (FileID, bool) so a genuine nil-UUID file is not mistaken for "nothing left". Model: opus-4-8 |
||
|
clawbot
|
b4654f8e52 |
Abort the run when packing fails, even under --skip-errors (closes #161)
A chunk is registered as pending (known, scanner-pending, packer pending-row) before it is packed. Under --skip-errors the scanner skipped a file on any processing error, including a failure inside addChunkToPacker (packing, database, encryption, upload). The pending chunk then stayed queued and a later blob finalize inserted it into the chunks table with no blob_chunks row, so a snapshot could complete holding a file whose chunk is in no blob and cannot be restored. Errors from addChunkToPacker are now marked and abort the run regardless of --skip-errors; only open and read errors are skipped. The bookkeeping order is unchanged. Flag help and comments now say only unreadable files are skipped. Model: opus-4-8 |
||
|
clawbot
|
39aef1c47c |
Stop config set echoing secrets; reject credential-bearing storage URLs (closes #166)
config set now prints only the key name after a write, never the value: a value may be a secret such as s3.secret_access_key, and echoing it leaks into captured stdout and pasted terminals. The set logic moves into writeConfigSet so this is testable. config set also tightens a pre-existing group- or world-readable config to 0600 after writing; the previous stat-and-preserve-mode block had no effect (os.WriteFile does not change an existing file mode) and is removed. ParseStorageURL now rejects s3:// and rclone:// URLs that carry credentials in the userinfo or an unknown query parameter, naming s3.access_key_id and s3.secret_access_key as where credentials belong. On a url.Parse failure only the inner cause is wrapped, so the raw URL is not echoed. file:// is unchanged. Model: opus-4-8 |
||
|
clawbot
|
96ebcd40d7 |
Reconcile purge against remote by hashed key, not human ID (closes #160)
syncWithRemote compared human snapshot IDs against the hashed metadata/<key>/ directory names, which never match, so it deleted every local snapshot record; the purge that followed then found nothing to remove remotely. Reconcile via listAllRemoteSnapshotKeys and RemoteSnapshotKey(id), matching CleanupLocalSnapshots, so a row still backed by remote metadata is kept. The purge tests only passed because their stubs used the human-ID layout production never writes; they now write metadata under the hashed remote key. New tests prove remotely-backed local rows survive the reconcile and that a purge removes the local row and remote metadata together. Model: opus-4-8 |
||
|
clawbot
|
d9f0220f94 |
Restore files at 0600 and make the blob hash check unskippable (closes #163)
Regular files are now created with O_EXCL at mode 0600 and given their stored mode only after the content is written and closed, so a file whose stored mode is restrictive is never briefly readable by other local users mid-restore. A file whose write or close fails is removed rather than left partial, and a chmod failure is a user-visible warning instead of a debug line. hashVerifyReader.Close now errors when closed before EOF, so a short read or early close can never obtain a blob whose hash was not verified; downloadBlobToCache drops the cache entry on any such failure. verifyFile (--verify) now rejects a restored file with bytes past its last chunk. Tests cover each behaviour under umask 022. Model: opus-4-8 |
||
|
clawbot
|
4c83e82543 |
Reject a blob_size_limit below the largest possible chunk (closes #167)
Validate only rejected blob_size_limit below chunk_size, but the chunker can emit chunks up to chunk_size times the FastCDC size spread (four times), and the packer puts a single chunk of any size into an otherwise empty blob. A limit between one and four times chunk_size therefore let a blob reach four times the configured maximum, with most blobs holding a single chunk and so exposing individual chunk lengths to anyone who can list the destination. Validate now rejects blob_size_limit below chunk_size times the spread, reusing the chunker's one constant (now exported as ChunkSizeSpread) instead of a second literal. The rule is stated in the error text, the Validate comment, the README config table, config.example.yml, and the generated config template. Model: opus-4-8 |
||
|
clawbot
|
86361c8b50 |
Fail closed on unreadable manifests instead of losing blobs (closes #157)
Prune learned which blobs are in use by reading every snapshot's manifest, but merely logged and skipped one it could not download or decode. Blobs referenced only by that snapshot then looked unreferenced and were deleted, with a zero exit -- and snapshot create --prune runs this unattended. collectReferencedBlobs now errors, naming the remote key, so prune deletes nothing and exits non-zero. Manifest generation likewise skipped a blob whose lookup failed or was missing, yielding a manifest short of what the snapshot needs; it now fails. Deep verify only warned when the manifest omitted a database blob; it now fails on any divergence. Docs corrected. Model: opus-4-8 |
||
|
clawbot
|
d77663d039 |
Default a scheme-less s3.* endpoint to TLS (closes #158)
With the s3.* config form and an endpoint written without a scheme, use_ssl being omitted built an http:// endpoint, while config.example.yml documented use_ssl as defaulting to true. Over plain HTTP a network observer sees manifests, object names, sizes and the access key id, and can alter responses. use_ssl is now *bool: omitted (nil) means the default, TLS; only an explicit use_ssl: false forces plain HTTP. This matches the s3:// URL form, which already defaults to TLS. The config init template dropped its misleading use_ssl line from the s3:// block (that key is never read for URLs; ?ssl=false controls TLS there) and points at ?ssl=false instead. Model: opus-4-8 |
||
|
clawbot
|
3abe9cbd9e |
Scope the PID lock to mutating commands (closes #150)
RunWithApp took the process-wide PID lock for every fx-backed command, so read-only commands (info, snapshot list, snapshot verify, remote info) failed with "already running" while a backup held it. AppOptions now carries a lockMode declared at each call site: only mutating commands (snapshot create, snapshot purge, snapshot remove, prune, remote nuke) acquire the lock; read-only ones run without it. snapshot restore is classified read-only -- it writes only to its target directory, not the local index or remote store. The decision moves to a small acquireLockIfMutating helper, with a test that a read-only command runs while the lock is held and two mutators still exclude. The README locking section is rewritten to match. Model: opus-4-8 |
||
|
clawbot
|
76a6917a35 |
Keep restore writes inside the target directory (closes #154)
restoreFile and verifyRestoredFiles joined the stored path onto the target with no containment check, so a ".." segment or an absolute path escaped the target, and a restored symlink could redirect a later child write anywhere on disk. Since age decryption proves a snapshot is readable but not honest, and restore usually runs as root, a forged snapshot became an arbitrary file write. Both call sites now go through containedRestorePath: it rejects a stored path unless filepath.IsLocal accepts it with the leading separator removed (barring "..", absolute, and empty paths), then Lstats each existing ancestor below the target and refuses to descend through a symlink. The target directory itself may be a symlink, and honest symlinks pointing outside the tree are still written verbatim. Model: opus-4-8 |
||
|
clawbot
|
38ebfd843a |
Trust only uploaded blobs for deduplication (closes #148)
An interrupted blob upload left the blob's chunks, blob_chunks, and blobs rows committed before the upload was attempted, so a later run deduplicated against data that never reached storage and produced a snapshot that reported success but could not be restored. Fix (issue option b): a chunk counts as known only when a blob holding it has uploaded_ts set, and each run drops un-uploaded blob rows and the chunks they orphan at startup, so the affected data is re-chunked and re-uploaded. A blob recorded with no remote backend is marked uploaded so the invariant holds uniformly. The reproduction is the interrupted-upload test from #72: its t.Skip is removed and it passes against this fix, and this branch's earlier duplicate copy is dropped. The interrupted metadata-export case is split to #177. Model: opus-4-8 |
||
|
clawbot
|
6b7517a4dc |
Add fault-injection tests for interruption and corruption (closes #72)
Adds internal/storage/faultstore, a storage.Storer wrapper that injects faults through the storage seam without patching production code: an upload that dies mid-stream, a backend reporting success while storing nothing, and reads returning corrupt or truncated bytes. Covers all six scenarios from the issue, each asserting the observable end state (index, destination, and what the user is told), not merely that an error returned. Scenario 1b (retry after an interrupted upload) exposed a real dedup defect and is skipped with a pointer to #148, which also owns the half-exported-state repair. Tests run serially because each calls log.Initialize on the global logger. No production behavior changes. Model: opus-4-8 |
||
|
clawbot
|
994e5de613 |
Quiet only the stdout UI under --json, not the log level (closes #112)
Per the decision on the issue (option 1), --json no longer implies Quiet. Folding --json into Quiet pinned the stderr log level to WARN, so prune --json gave a machine consumer no record of the local index rows it deleted. The two effects are now split: a JSON field on log.Options drives only the stdout UI-quiet in setupGlobals, keeping the JSON document clean, while the stderr log level follows --verbose/--debug again (diagnostics have gone to stderr since #82). The same coupling is removed for snapshot verify, snapshot remove and remote info; snapshot list was already decoupled. Model: opus-4-8 |
||
|
clawbot
|
42f4e648d7 |
Reconcile docs with code across the accuracy-sweep items (closes #74)
Docs-only sweep of the accuracy items. Corrected ARCHITECTURE.md chunk sizes and the fx config type; documented the ls/rm aliases, the CPU/MEM profile env vars, the age_secret_key threat-model caveat, the four zstd presets, and a new locking section for the process-wide PID lock. Narrowed the internal/ui output claim to what holds today (refactor deferred to #149); lock-scoping deferred to #150. Added the missing ARCHITECTURE.md and config.example.yml README links. Every claim re-verified against the tree. Disclosure: a pre-existing gomodguard linter deprecation surfaced during the gate; unrelated. Model: opus-4-8 |
||
|
clawbot
|
343129f891 |
Accept a remote key for restore and verify, and document it (closes #124)
A machine restoring after the original is gone has no local index and cannot know a snapshot's human ID; snapshot list shows such snapshots only by their remote key, but restore and verify accepted only the human ID, so recovery could not be done as documented. Restore and verify now also accept a remote key, or an unambiguous leading part of it as snapshot list prints it, resolved against the store's metadata listing. Human IDs are never pure hex, which tells the two forms apart. Deep verify reads the single snapshot in the downloaded per-snapshot database. A new README section walks the recovery end to end; a test backs up, then lists, restores and deep-verifies with an empty index, another hostname and no age_recipients. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
a50e3fa038 |
Add tests for internal/storage: URL parsing, backends, shared conformance suite (closes #66)
internal/storage, the package that parses store URLs and selects the backend, had no tests. Adds table-driven tests for URL parsing (each scheme, query parameters, malformed input, unknown scheme, backend type chosen); one shared conformance suite for the Storer interface, run against the file backend in a temp directory and the s3 backend on the in-process harness internal/s3 already uses, so a new backend inherits it; and rclone construction and argument tests using its in-process local backend. A comment records that rclone data operations need a configured remote and are not unit-tested. No production code changed and no defect surfaced. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
6fcd8e1668 |
Stamp Docker image version from the host; flush profiles on error exit (closes #75)
Docker images reported commit unknown because the build ran git inside the container while .dockerignore excludes .git, and VERSION was never overridden. script/docker and script/cibuild now compute version, commit and date on the host and pass them as build args; the Dockerfile runs no git and falls back to dev and unknown, never empty, on a bare docker build. Profiling a failing command gave a truncated or missing profile: Entry and each command goroutine called os.Exit(1), skipping the deferred profile writers in main. Entry now returns a status that main exits with after its defers run, and command goroutines report failure through one RunOperation helper, which also restores PID-lock release and graceful shutdown on failure. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) Co-authored-by: clawbot <clawbot@noreply.example.org> |
||
|
clawbot
|
aab6a87f8c |
Reconcile the schema/migration docs with the code (closes #68)
Four documents told different stories about the database schema. docs/DATAMODEL.md now owns the explanation and separates two things: the policy, which is unchanged (no supported upgrade path between versions; delete the local index with vaultik database delete and back up again), and the schema bootstrap that does exist (numbered files in internal/database/schema applied to a fresh database and recorded in schema_migrations). README.md and AGENTS.md are reworded to match and link there. AGENTS.md names the real file to edit, internal/database/schema/001.sql, and notes that the pre-1.0 disposability clause expires on tagging. No code changed. Judgement call: REPO_POLICIES.md still names a different schema file; it is cross-project policy and was left alone. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
c355ef4d25 |
Report a prune count that could not be read as unknown, not 0 (closes #96)
Prune read table row counts before and after to report how many orphaned files, chunks and blobs it removed, and discarded the error from every read. A failed query therefore reported as a count of 0, and the summary showed plausible wrong numbers. A count that cannot be read is now logged as a warning (on stderr, also under --json) and shown as "unknown"; a difference computed from an unknown count is itself unknown. 0 still means the table was empty. No --json document carries these counts, so none can show a false 0. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
5927e1aa3d |
Write file:// blobs atomically via temp file and rename (closes #130)
The file:// backend streamed each object straight to its final key, so an upload cut off mid-stream left a truncated object there. The next backup saw that Stat succeeded, recorded the blob as complete, and produced a snapshot that reported success but could not be restored. Writes now go to a temporary file with a .partial suffix in the destination directory, are synced, then renamed onto the key. List and ListStream skip .partial files, so a leftover is never trusted as a blob and is overwritten when the key is written again. S3 PutObject is already atomic. Disclosure: the containing directory is not synced after the rename, so a host crash right after it could still lose the object on some filesystems. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
9ca962969a |
Map s3 not-found to storage.ErrNotFound in Get and Stat (closes #129)
The Storer interface documents that Get and Stat return storage.ErrNotFound for a missing object. The file and rclone backends did; the s3 backend returned the raw SDK error, so callers testing for ErrNotFound behaved differently on s3. S3Storer.Get and Stat now wrap ErrNotFound when the SDK reports a missing object and leave every other error untouched. The SDK reports a missing key two ways (NoSuchKey from Get, NotFound from Head); both are recognised in one helper, s3.IsNotFound, which HeadObject now also uses. The mapping lives in the storage package because internal/s3 cannot import it. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
89ebfc78e2 |
Use one duration parser and fix the --older-than months example (closes #123)
Two parseDuration functions existed with different grammars; only the one in internal/vaultik/helpers.go was reachable from a flag. The unused copy in internal/cli/duration.go is deleted, so no flag accepts anything it did not before. The README gave 6m as the six-months example for snapshot purge --older-than, but m is minutes: that command removed every snapshot older than six minutes. The example is now 6mo, and the help for --older-than and --keep-newer-than states that m is minutes and mo is months. The parser now rejects negative durations, which it used to accept or silently make positive. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) Co-authored-by: clawbot <clawbot@noreply.example.org> |
||
|
clawbot
|
07ef3a1c78 |
Drop the lint-guard shell scanner, keep the Dockerfile.lint checks (closes #121)
The guard test in cmd/vaultik/lintdocker_test.go tried to prove that no script runs the linter outside the container by parsing shell scripts with a hand-written scanner. Four reviews each found another spelling it missed; such a parser cannot be complete, and nobody could follow it in one reading. The scanner, its helpers and their tests are deleted. The plain Dockerfile.lint assertions stay: the linter image is pinned by digest, config verify runs before run, and the per-run value reaches both steps. TODO.md no longer claims a test proves the property; script/lint is the only lint entry point, and keeping it so is a review matter. Judgement call: this drops a guard two reviewers asked to harden. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (decision, merge) |
||
|
clawbot
|
c423d13191 |
Hash the plaintext, not the encrypted bytes, in verify --deep (closes #131)
The last step of verify --deep hashed the encrypted bytes it downloaded once with SHA256 and compared the result to the blob name. The name is the double SHA256 of the blob plaintext, so the two could never match and deep verification failed on every healthy blob with "blob hash mismatch". It now hashes the decompressed plaintext as chunk verification streams it and compares the double SHA256 of that to the blob name, the same derivation the writer uses. A new test backs up a real snapshot, runs deep verify on it, then flips one byte in a stored blob and expects failure. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
75a10d3a22 |
Hash-verify the Go toolchain in the release workflow (closes #105)
The release workflow installed Go with actions/setup-go, which pins the action but not the Go archive it downloads, so the compiler that builds the published binaries was verified against nothing in this repo. New script/install-go, modelled on script/install-goreleaser, downloads the go.dev archive for the version in go.mod and refuses it unless its sha256 matches the value committed in the script. It fails if its version disagrees with go.mod, and on any OS or architecture other than the Linux release runners. GOTOOLCHAIN=local on the release step keeps the verified toolchain from switching itself. Judgement call: release path only; script/bootstrap still uses the host Go. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
3d56dd7eb0 |
VACUUM snapshot metadata through the sqlite driver, not a CLI (closes #120)
snapshot create compacted the metadata database by running a sqlite3 command-line binary, after every blob had already been uploaded. On a host without that binary, which includes anyone who installed with go install, the backup failed at the last step, and two tests failed the same way. VACUUM now runs through the Go sqlite driver the program already uses, and its error is returned to the caller. The runtime Docker image no longer installs the sqlite package, since nothing in the binary calls it. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
bdce350041 |
Delete dead code and stale fixtures, fix config set reindent (closes #70)
Removes code and fixtures nothing uses: the internal/models package and its test, a second SnapshotInfo type in package cli that had no references (the live one is in internal/vaultik), and two config fixtures, test-config.yml and test/integration-config.yml, whose keys the config loader no longer accepts. test/config.yaml stays; a test uses it. Also fixes config set, which rewrote the whole file with 4-space indentation on the first set despite the documented promise to preserve formatting. It now encodes with 2-space indent like the default template, and a test asserts comments and indentation survive a set. Deviation: one commit, not one per deletion as the issue asked. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
753bc3ef60 |
Correct remote layout and privacy docs for hashed snapshot keys (closes #67)
Three documents showed the remote layout with a plaintext snapshot ID as the metadata directory name, and docs/REPOSTRUCTURE.md blamed those IDs for the observable backup time. The store actually names each metadata directory with a one-way hash of the ID, so hostname and snapshot name are not visible; the backup time is, through the plaintext timestamp in the manifest, which is accepted behaviour. README, ARCHITECTURE.md, docs/DATAMODEL.md and docs/REPOSTRUCTURE.md now show the hashed layout, the derivation is documented once, and the privacy section lists what the unencrypted manifest exposes. Two code comments that claimed the timestamp was hidden are corrected. No behaviour change. Judgement call: docs/DATAMODEL.md was not named in the issue but had the same error. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) |
||
|
clawbot
|
d2a0510cb4 |
Trigger CI on next, not only main (closes #122)
`check.yml` ran only on push to `main` and on pull requests against `main`. Every unit is a PR based on `next`, and `next` is pushed on each squash-merge, so no unit PR and no push to `next` ever ran CI; a broken `next` would first surface on the milestone PR. `next` is added to both branch lists; nothing else in the workflow changes. The README Entrypoints section now says where CI runs. Disclosure: the CI run on the PR itself fired (the proof the trigger works) but was red because the runner had no disk space left before any check step ran; the local gate was green. Model: opus-4-8 (implementation and review) model: claude-fable-5 |
||
|
clawbot
|
583f65040a |
Correct --cron flag help to name warnings as unsuppressed (closes #87)
check / check (pull_request) Failing after 0s
`--cron` sets the UI quiet, but `Warningf` and `Errorf` are unconditional, and the snapshot summary is routed through `Warningf` on purpose so cron delivers something on a successful run. The help string said `silent unless error`, so a user could read normal cron output as a failure. It now says `silent unless warning or error`, matching the README. String only; no behavior change. Model: opus-4-8 (implementation and review) model: claude-fable-5 |
||
 sneak
|
d257f8f658 |
Lint in a container as a build step, via Dockerfile.lint (closes #113)
check / check (pull_request) Successful in 2m58s
Every lint run now happens inside its own container, invoked through script/lint, and linting is a build step rather than a container command: a successful build of the new root Dockerfile.lint IS a clean lint. That shape also works where the docker daemon is remote and bind mounts are impossible. Its FROM line -- golangci/golangci-lint:v2.12.2, pinned by digest -- is now the only pin of the linter version in this repo. A container per run has its own lint cache and its own golangci-lint lock, both discarded with it, so neither cross-worktree contamination nor lock contention exists any more. The machinery that defended against them is therefore gone: the per-worktree cache directories, the lock-retry loop, and script/lint-audit, which existed to catch findings replayed from a cache that no longer exists. So is the host lint path in its entirety -- the native escape hatch, its version detection, and VAULTIK_LINT_IN_CONTAINER in both script/lint and the Dockerfile. Nothing lints on the host, at any version. A cached build lints nothing, so the CHECK_EPOCH mechanism the product Dockerfile already used is what makes a green mean something: ARG CHECK_EPOCH with no default, placed below the module layers so dependency caching survives, a `RUN [ -n "$CHECK_EPOCH" ] || exit 1` guard so a build that withholds the arg fails instead of replaying, and the value expanded into the lint command itself. script/lint computes `epoch="$(date +%s%N)$$"` as a bare assignment on its own line, because inline in the argument a failing substitution does not abort under `set -eu` and yields a constant empty epoch -- which is exactly the false green being prevented. The product Dockerfile loses its lint stage rather than gaining a second linter pin. That stage ran `make lint`, which is now `docker build`: docker-in-docker inside a BuildKit step with no daemon. Calling golangci-lint directly there instead would have meant two independently bumpable digests for one tool. `make fmt-check` moves beside `make test` in the builder stage, and script/cibuild now builds Dockerfile.lint and then Dockerfile, each with its own fresh epoch, failing on either. Consequence, stated in comments rather than left to be discovered: script/docker builds the product image only and no longer lints; script/check and script/cibuild are the gates. `golangci-lint config verify` runs as its own epoch-keyed layer, above the lint. It is not belt-and-braces: `golangci-lint run` rejects a config it cannot PARSE but silently IGNORES an unknown top-level KEY. Renaming .golangci.yml's `linters:` to `linterz:` -- one character -- discards `default: all`, the disable list and every threshold, leaves only the small default linter set running, and exits 0 reporting `0 issues.` on a tree the real config fails with an lll finding, in a run whose lint layer demonstrably executed. That is a set-but- ineffective config falling back to defaults instead of failing loudly, sitting in the gate's own configuration. `config verify` catches it and does so with the network genuinely off at this pin: under `docker run --network none` against the pinned digest it exits 0 on this repo's config and exits 3 on the `linterz:` variant. It is keyed on CHECK_EPOCH like the lint itself, because a cached validation validates nothing. script/lint-fix is kept, reimplemented as a bind-mounted docker run against the image parsed out of Dockerfile.lint -- a build step cannot write fixes back to the worktree -- and its header states outright that it is a developer convenience, never a gate, and needs a local daemon. cmd/vaultik/lintdocker_test.go parses both Dockerfiles and both scripts and fails if any part of the mechanism is dropped: the digest pin, the defaultless ARG below `go mod download`, the emptiness guard, the expansion of the epoch into each check command, the bare per-invocation epoch assignment in both scripts, cibuild building both files, the config verification running before the lint, and -- structurally, not by searching for one retired variable name -- that no script invokes golangci-lint except through docker. Every one of those losses is silent: the build still exits 0 and nothing is checked, which is why they are asserted rather than trusted. The scanner behind the last of those has its own test, because a structural check that goes blind passes on every tree, including a broken one. script/lint takes no arguments now, and says so instead of dropping them: a build step has no command line to pass linter flags to. |
||
|
clawbot
|
696ed9ab4d |
Gate prune's local-cleanup output on --json, and make make build build (closes #108)
check / check (push) Successful in 2m16s
Closes #110. CleanupLocalSnapshots wrote three prose lines to stdout with no --json awareness, covering every branch, so `vaultik prune --json | jq` failed on any input. -q never helped either: printlnStdout and stdoutf write straight to v.Stdout and never consult v.UI, which is what SetQuiet affects. It now takes *PruneOptions, symmetric with its sibling phase PruneBlobs, and gates all three writes. Threading opts.JSON was chosen over moving the lines to log.Info, because internal/log/log.go defaults the level to Warn: log.Info would not have relocated them to stderr, it would have deleted them from a plain `vaultik prune`, and "Removing stale local record" narrates the deletion of local index rows. The stale-record count is deliberately not added to PruneBlobsResult - every field there is blob-scoped and produced by the phase that runs after this reconciliation, so adding it would change a published --json schema as a side effect of a stream fix. Note for anyone reading the --json contract: under --json the stale-record removal now produces no signal in either stream. stdout is correctly gated, stderr is level-pinned to Warn because --json sets Quiet, and the count is not in the document. That is inherited behaviour - PruneBlobs' own log.Info calls are equally invisible under --json - not something this change introduced, and it is tracked separately. make build exited 0 and produced nothing: .PHONY listed build with no build: rule, and a phony target with no prerequisites and no recipe is considered already satisfied, which turns what would be a hard error into a silent success. In a repo where `make build` is the documented way to build, a caller checking the exit code concluded the build worked. Now `build: vaultik`, verified in both directions - a clean build produces the binary, a deliberately broken one exits non-zero and produces none. All 19 .PHONY names were audited; build was the only one lacking a rule. TestPhonyTargetsAllHaveRules keeps that true for names added later, so the class is closed rather than the instance. |
||
|
clawbot
|
f21e7c9e70 |
Suppress the startup banner for --json (closes #106)
check / check (push) Successful in 2m31s
The banner is printed to stdout before cobra parses, and bannerSuppressedInArgs recognised only --quiet, -q and --cron. So every --json document was preceded by two banner lines and a blank one, and `vaultik snapshot list --json | jq` failed. Passing opts.JSON as extraQuiet did not help: that calls UI.SetQuiet in an fx OnStart hook, long after Entry has printed. The raw-argv scan is extended rather than the banner moved after parsing. root.go documents that the banner must survive cobra rejecting its arguments and --help, and no single post-parse location covers those paths. The subcommand-versus-persistent distinction does not decide it: --cron is already in the suppression list and is itself subcommand-only, existing on snapshot create alone, so this adds another instance of an accepted imprecision rather than a new kind. The error directions are asymmetric - a false positive loses a decorative banner, a false negative corrupts a document - so the scan errs toward suppression, which is also why --json=false suppresses, exactly as --quiet=false already does. Four of the five --json commands now pipe into jq cleanly with no other flags: snapshot list, snapshot verify, snapshot remove, remote info. prune does not, because pruneLocalSnapshots writes three prose lines to stdout with no --json awareness. That reproduces identically before this change and -q never suppressed it either, since printlnStdout and stdoutf bypass v.UI entirely. Tracked as #108. Also fixed: TTYHandler's human-readable byte formatting did not survive grouping, because the key check compared against the bare attribute name and a grouped record presents it qualified. AGENTS.md policy 9 keyed the log format on stdout's TTY-ness, which #82 made false by moving the logger to stderr; it now names the log stream. Vaultik.Stderr keeps its field with the comment amended to say outright that nothing writes to it, and the dead listEnv.stderr is removed. |
||
|
clawbot
|
c16ef476a9 |
Log to stderr and stop discarding With attributes (closes #82)
check / check (push) Successful in 4m20s
Closes #97. internal/log attached both handlers to os.Stdout, so any record that was not suppressed landed in the middle of a --json document. WARN and ERROR are never suppressed, so this was not hypothetical: a config file with permissions looser than 0600 was enough to break `vaultik snapshot list --json | jq`. Both handlers now write to os.Stderr, and the TTY-vs-JSON format choice tests os.Stderr rather than os.Stdout - the format has to follow the stream the records land on, or a redirected stderr gets colorized whenever stdout happens to be a terminal. User-visible: --verbose and --debug output moves to stderr too, so `vaultik snapshot list -v > out.txt` no longer captures diagnostics. --quiet and --cron semantics are unchanged. TTYHandler.WithAttrs and WithGroup discarded their arguments and returned the receiver, while their doc comments claimed otherwise, so attributes passed through the exported log.With vanished. The effect was environment-dependent in the worst direction: handler choice is by TTY-ness, so attributes disappeared on a terminal - where a developer is debugging - and appeared correctly in CI. Both now return a new handler with copied state rather than mutating the receiver, since slog permits a handler to be shared and derived from concurrently. A test asserts the TTY and JSON handlers emit the same attribute set, which is the test that would have caught the original defect. The local workaround in snapshot_list.go is removed now that the logger no longer writes to stdout. The collect-then-emit machinery is kept, but for a different reason than it was added: emitting from the fetch workers would order warnings by network timing, whereas key-order emission after group.Wait() is deterministic run to run. Not yet complete: --json stdout still carries the startup banner, which internal/cli/entry.go writes before cobra parses and which bannerSuppressedInArgs does not recognise --json for. That is the remaining stdout contamination path and is tracked in #106. |
||
|
clawbot
|
e3f407b440 |
Make the tagged-release path work on Gitea (closes #65)
check / check (push) Successful in 3m7s
No tag could be cut at all: .goreleaser.yaml had no gitea_urls block, so
goreleaser defaulted to the GitHub API, and the repo has zero tags.
.goreleaser.yaml now points at git.eeqj.de. Version derives from git via
a new script/version - exact tag with any leading v stripped, else
dev-<12-char sha>, with a -dirty suffix when tracked files are modified -
replacing the hardcoded 1.0.0-rc.1 that every local build was stamping
regardless of git state. A tag-triggered .gitea/workflows/release.yml
runs goreleaser with a scoped token (RELEASE_TOKEN); script/bootstrap
installs a sha256-verified goreleaser, and make release / release-snapshot
become script shims like every other target.
Two fabrications were removed rather than merely replaced. goreleaser's
snapshot.version_template was `{{ incpatch .Version }}-next`, which
invents a release number from the last tag - and with no tags, from
goreleaser's own fabricated v0.0.0. And internal/cli/version.go gated its
development-build notice on Version == "dev" exactly, so the moment
untagged builds carried a sha that notice would have gone silent and an
unreleased binary would have read as a release. Replaced with a tested
IsDevVersion predicate, and closed at both layers: the Makefile now
refuses to build when script/version yields nothing, and an empty version
counts as a development build - reachable today via
`docker build --build-arg VERSION=`.
The release workflow installs Go from a sha-pinned actions/setup-go
(v5.6.0) using go-version-file, so the compiler that produces released
binaries is pinned like every other external reference. Without it the
first tag push would either fail at goreleaser's before-hook or compile
the published artifacts with whatever unpinned Go the runner happened to
carry - the one unpinned thing in a release path that already refuses an
unpinned goreleaser.
Known gap: the Go tarball setup-go fetches is version-pinned but not
checksum-verified against a value in this repo, unlike the goreleaser
install and the Dockerfile digest.
|
||
|
clawbot
|
b6e4a218a3 |
Isolate the lint cache and context-gate the native path (closes #99)
check / check (push) Successful in 2m23s
Closes #80. script/lint pointed GOLANGCI_LINT_CACHE at a path shared by every worktree of this repo. Two worktrees have identical Go file contents, so their cache keys collided and one tree's stored findings replayed for another, paths included - observed as 231 findings all citing another session's worktree, with no parallel-runner message to signal it. The failure is symmetric and only one direction is loud: a clean tree failed by a dirty sibling gets investigated, a dirty tree passed by a clean sibling does not. The cache is now keyed per worktree on a digest of $ROOT, and remains persistent. Independently of that, script/lint-audit inspects every run's output and fails the run if any finding cites a path outside the tree being linted. That guard is the load-bearing part: it converts a silent unearned green into a hard error regardless of how the cache is keyed. It is deliberately built so it can never certify a pass, only reject, so it cannot itself become a gate that reports green. The native path was gated on version equality alone, which admitted a locally installed matching binary and bypassed the digest pin. It now requires VAULTIK_LINT_IN_CONTAINER=1, set only by the Dockerfile lint stage, in addition to version equality. /.dockerenv was rejected as the signal because dockerd creates it for `docker run` but not reliably during a BuildKit `docker build`, which is the case the exception exists for. A version mismatch inside the container is now a hard error rather than a fall-through. This mattered more than the issue supposed: on this host a matching golangci-lint exists on PATH, so script/lint was taking the native path and linting against the global cache without ever running the pinned image. That is the likely root of the observed contamination, and it is closed here rather than mitigated. The parallel-runner error is retried rather than reported. It is not a lint result, and surfacing it as a non-zero exit is indistinguishable to a caller from real findings; exhausted retries fail saying the tree was never analysed. Note that a private cache alone does not remove lock contention - measured with two concurrent runs using separate cache directories. script/bootstrap no longer reports success on a machine that cannot run the gate: docker is now required by lint, check and precommit, so a missing binary or unreachable daemon is a hard failure naming what will not work. |
||
|
clawbot
|
c51f693527 |
Make the test gate unfakeable and stop test-integration lying (closes #93)
check / check (push) Successful in 3m42s
Closes #69. script/test ran `go test` without -count=1, so Go's test cache satisfied the gate without running anything: a repeat `make test` printed all 14 ok lines in 0.42 seconds, every one marked (cached). Those lines count as ok lines, so the evidence signal this repo relies on was forgeable. It sits below the Docker layer cache - CHECK_EPOCH forces `RUN make test` to re-execute, but a GOCACHE baked into an earlier image layer survives into the re-executed step, so the step can run and still do no work. -count=1 is applied unconditionally rather than only in the container, because the pre-commit hook runs the same script and a gate honest only in CI is dishonest where it is leaned on most. It costs about 11 seconds on every repeat run, which is what it costs for a repeat run to mean anything. test-coverage had the same omission and is fixed too; a coverage profile assembled from cached results describes a run that did not happen. Both invocations in script/test now share one run_tests function so the quiet run and the verbose rerun cannot drift apart in flags. make test-integration passed -tags=integration while no file in the repo carries any build tag, so it was an exact duplicate of make test. Removed rather than given a tag scheme: the whole suite is 12s on the host, so gating saves seconds in exchange for a mechanism whose failure mode is "some tests silently stopped running" - a poor trade in a repo that has found several ways for a gate to report an unearned green. -timeout goes 30s to 120s. This DIVERGES from REPO_POLICIES.md:192, which mandates 30s; the divergence is deliberate, recorded in script/test's comment, and proposed upstream as #101. Measured worst case is 10.2s and each fresh measurement has come in above the last, leaving 30s at 2.9x - too thin for a loaded runner. A -timeout is a hang backstop, not a performance budget. Note for the record: cold-cache compilation is NOT charged against -timeout. The flag reaches the test binary as -test.timeout and its clock starts inside testing.M.Run, after compilation. Verified twice independently - a run with an empty GOCACHE spent ~46s compiling and then reported per-package durations within noise of warm. A shell `timeout 30 go test ./...` does include compilation, but that is a different mechanism. |
||
|
clawbot
|
3f9c2e5033 |
Record the stale-branch triage and advance TODO.md (closes #71)
check / check (push) Successful in 2m5s
Twelve stale remote branches retired. Nine were ancestors of main with no unique commits; golangci-v2.12.2 pointed at a tree byte-identical to the one main's |
||
|
clawbot
|
50816b7415 |
Make a missing CHECK_EPOCH fail the build (closes #91)
check / check (push) Successful in 3m2s
PR #89 stopped script/cibuild replaying cached check layers, but left a gap: a bare `docker build .` with no --build-arg still faked. An unset ARG is an empty string, an empty string is a stable cache key, and the check layers replay from it. That gap mattered because REPO_POLICIES.md names `docker build .` verbatim as a command that must be green, so the documented command was the one that lied. Both check stages now carry `RUN [ -n "$CHECK_EPOCH" ] || exit 1` immediately under their own ARG. Failed steps are never cached, so this fails on every invocation rather than once - a bare build now stops with a named error instead of reporting a green it did not earn. Each stage needs its own guard because ARG scope is per-stage; a gate-carrying stage without one is a silent hole if ordering ever changes. The check RUNs now reference the value (`echo "check epoch: ${CHECK_EPOCH}" && make <target>`), so the cache miss is contractual rather than resting on BuildKit's current treatment of unreferenced ARGs, and the epoch is visible in the build log. The epoch becomes "$(date +%s%N)$$" so concurrent invocations in the same second cannot collide. busybox silently drops %N and exits 0, so $$ is what makes it correct there. The bare-assignment form is retained deliberately: inlining the substitution into --build-arg would, under set -eu, yield an empty and therefore constant epoch without aborting. script/docker gets the same treatment - it is not the gate, but two entrypoints disagreeing about whether the tree is green is its own hazard, and local builds are almost always warm. Verified by negative control rather than inspection: a bare build fails twice consecutively here and succeeds twice on the parent commit, so the change is demonstrably not a no-op. The builder-stage guard was fired directly with a targeted probe build, since the lint stage otherwise fails first and would leave it unexercised. |
||
|
clawbot
|
c3bb3b5580 |
Make script/cibuild unable to report an unearned green (closes #85)
check / check (push) Successful in 3m13s
script/cibuild was a bare `docker build .`. On an unchanged tree Docker served the check RUN layers from cache, so make fmt-check, make lint and make test never executed - and the build still exited 0. Measured at 221ms with zero ok lines and every check layer CACHED, against 162s for a real run. CI showed the same signature: 6 second "successes" on main. An ARG CHECK_EPOCH now sits immediately above the check RUNs in both stages - each stage declares its own, since ARG scope is per-stage - and script/cibuild passes a fresh value per invocation. Dependency and module layers sit above the ARG and still cache, so this does not make every build cold. The epoch is assigned before the build rather than inlined into the --build-arg. Under `set -eu` a command substitution that fails inside an argument does not abort the script: CHECK_EPOCH would become an empty string, an empty string is a constant, a constant CHECK_EPOCH restores the cached false green, and the guard would silently disarm itself while still exiting 0. As a bare assignment, set -e catches a failing date and no build starts. The README and Dockerfile state the guarantee conditionally. It holds per build context and CHECK_EPOCH value, and depends on script/cibuild passing a fresh one - a bare `docker build .` with no --build-arg still replays the check layers from the second consecutive run onward. That residual gap is tracked in #91 along with the remaining upstream hardening. Verification is recorded once, in the PR's verification comment, rather than restated with differing numbers in three places. |
||
|
clawbot
|
3bcdbcfd83 |
Correct what --cron actually suppresses (closes #84)
check / check (push) Successful in 6s
The Vaultik.UI doc comment claimed the cli layer replaces the writer with
a discarding writer in --cron mode. It does not. UI is built once as
ui.New(os.Stdout) and never reassigned; internal/cli/app.go calls
UI.SetQuiet(true) when --cron or --quiet is set, which drops Begin,
Complete, Info, Notice, Detail, Progress and Banner - but Warningf and
Errorf have no quiet check and are still emitted.
That distinction matters: the end-of-run summary is deliberately routed
through UI.Warningf so cron delivers something, so a reader who believed
the comment would have concluded the opposite of how the code is meant to
work.
The README's --cron description carried the same imprecision ("Silent
unless error") and is corrected alongside it.
Comment and documentation only - the Go diff contains no non-comment
lines, so there is no behavior change.
|
||
|
clawbot
|
50e20b460e |
List remote snapshots without requiring the private key (closes #64)
check / check (push) Successful in 6s
ListSnapshots built its table entirely from the local SQLite index. The only remote access, reportRemoteDrift, was gated on AgeSecretKey != "", so on a correctly configured host - which by design holds no private key - snapshot list never contacted the destination store at all. A user who lost their local index could not see their own backups, and the "<remote only>" cell the README documents was unreachable dead code. The listing is now the union of the local index and the destination store, with no age_secret_key gate. Remote-only snapshots cannot have their hostname or name recovered - RemoteSnapshotKey is one-way and the manifest stores the hash - so they are listed by abbreviated remote key with the real timestamp and compressed size from the manifest, and "<remote only>" in the two columns that require the local index. Nothing new is written to remote storage and the human ID is never fabricated. An unreachable destination degrades to local-only with a warning and a zero exit code. remote_present is null rather than false in that case, so "absent" and "unknown" stay distinguishable and no drift is claimed from a listing that never happened. Also: - Snapshot timestamps are normalised to UTC in scanSnapshotRows, the single point where they enter the domain. Previously one of three scanners omitted .UTC(), so on a non-UTC host the same snapshot rendered a different time depending on whether it was locally tracked. - The 1000-row cap and the unreadable-manifest count are reported in --json mode as well as table mode, so machine consumers cannot be silently truncated. The JSON shape is unchanged. - Warnings raised while listing are routed to stderr rather than the logger, which writes to stdout and would corrupt the JSON document. This is a local workaround for the logger bug tracked in #82 and should be removed when that lands. - downloadManifestByKey is now the only remote manifest reader, so the manifest privacy question in #81 has a single call site to change. - The orphaned "vaultik snapshot cleanup" hint now names vaultik prune; that command was folded into prune by the 2026-07-02 consolidation. |
||
|
clawbot
|
af607e3597 |
Run the linter at the pinned version locally too (closes #78)
check / check (push) Successful in 6s
script/lint ran bare golangci-lint from PATH while CI and the Dockerfile pinned v2.12.2 by digest, so make lint and CI could disagree about findings. That drift ran both directions: it produced two false green claims during the lint remediation, and on an ambient 2.10.1 it also reported four gosec findings on a tree CI linted clean. script/lint now extracts the image reference - tag and digest - from the Dockerfile lint stage FROM line and runs that exact image under docker. The Dockerfile FROM line is the single source of truth for the linter version; the duplicate pins in the Makefile deps target and in script/bootstrap are removed rather than kept in sync. A golangci-lint on PATH is used only when its version exactly equals the pin, which is what makes the in-container lint stage work (the Dockerfile runs make lint inside the pinned image, where there is no docker daemon). Any other version, or none, goes through docker. When docker is unavailable the script fails with an actionable message and never falls back to a different linter version. script/lint-fix delegates to script/lint --fix so autofixes come from the pinned linter too. The container mounts persistent build and module caches and runs as the invoking uid/gid. Verified by reinstating the four historical nolint directives that 2.10.1 requires and 2.12.2 reports as unused: the old script passed on that tree and the new one fails with four nolintlint findings. |