A blob hash read back from the downloaded snapshot database or the store listing was trusted unchecked. A hostile remote could set a hash such as "aa/../../etc" and have a decrypted blob written outside the cache directory, or feed a short or negative value that panicked a command.
blobDiskCache.path now refuses any key with a path separator, and ReadAt rejects a negative offset or length, bounding so a sum cannot overflow past the check. A new isBlobHash helper gates FetchBlob, shallow and deep verify, and restore: buildBlobIndexes rejects every hash from the snapshot database before any fetch. The blobs/ and metadata/ listings skip a non-conforming name, and short-hash prefixes in log and error text go through a panic-safe shortHash helper.
Model: opus-4-8
Clears the final 80 golangci-lint findings under the canonical
.golangci.yml (sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb),
taking the repo from red to green: script/cibuild exits 0.
- wsl_v5 (60): blank line above defer/go statements sharing no variable
with the line above; blank-line-only diff.
- sqlclosecheck (10): the package-local CloseRows helper hid the close
from the analyzer. Helper removed; all 18 call sites now defer an
inline rows.Close(), preserving the fatal-on-close-error path. No
resource leak existed - the rows were always being closed.
- prealloc (3): append targets given a starting capacity.
- revive (3): package-name findings suppressed with per-site directives
pending the naming decision tracked in #76.
No gosec suppressions are needed under the pinned linter. .golangci.yml,
Dockerfile, Makefile, .gitea/ and script/ are byte-identical to main.
Verified with script/cibuild (digest-pinned golangci-lint v2.12.2), not
make check - the latter resolves the linter from PATH and is not a
trustworthy gate here; see #78.
Closes#59.
Updates golangci-lint to v2.12.2 everywhere it is pinned and installs the canonical `.golangci.yml`, then remediates every finding the new linter/config surfaces so `make check` is green.
## Version bump
- `Dockerfile` lint stage: `golangci/golangci-lint:v2.11.3-alpine` -> `v2.12.2-alpine` (digest-pinned, date comment updated)
- `Makefile` `deps` target: `go install` moved from the old v1 module path at `@latest` to the pinned `github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2`
- `.golangci.yml` replaced with the canonical config (v2 schema; settings under `linters.settings` so the thresholds actually apply; `default: all` with the standard six disables)
- `script/bootstrap` installs golangci-lint via the system package manager and carries no version pin, so it is unchanged
- CI (`.gitea/workflows/check.yml`) only runs `script/cibuild`, so it needed no change
## Lint remediation
The canonical config surfaced ~3,300 findings across 56k lines. All are fixed, behavior-preserving; incorporates and supersedes the per-package mechanical passes already merged to `main` (refs #61). Highlights:
- `err113`: dynamic errors replaced with package sentinels + `%w` wrapping; comparisons via `errors.Is`
- `goprintffuncname`: printf-style helpers renamed with an `f` suffix (`ui.Writer` message methods, `cli.ReportErrorf`, `database.Fatalf`) and all call sites updated
- `revive` stutter renames: `blob.Handler`, `blob.WithReader`, `blob.ChunkPosition`, `storage.URL`, `storage.Info`; missing doc comments added
- `contextcheck`/`noctx`: `context.Context` threaded through `blob.Packer` and the scanner call sites; context-aware `exec`/`sql` variants
- `funlen`/`cyclop`/`gocognit`/`dupl`: oversized and duplicated functions split into focused helpers (production and test code)
- tests: `t.Parallel()` added where safe (global logger init kept in the serial phase for `-race`), `t.TempDir()`/`t.Helper()` adopted, several suites converted to external test packages
- `gosec`: bounded integer conversions, `ReadHeaderTimeout` on the test HTTP server; remaining warnings suppressed per-site with justifications
- remaining `nolint` directives are rare, targeted, and each carries a reason (e.g. `nilnil` not-found contract in the repository layer, fx module globals, on-disk snake_case struct tags)
- removed the deprecated `log.LogOptions` alias (callers migrated to `log.Options`)
`make check` (tests with `-race`, lint, fmt-check) passes.
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #62
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
The CLI had two commands named "prune" doing different jobs (local
DB orphan cleanup vs. remote blob garbage collection), which was
confusing and forced a manual two-step workflow after deleting any
snapshot.
Single user-facing prune surface is now `vaultik prune`, which calls
PruneDatabase (local orphan cleanup) then PruneBlobs (remote unref
blob GC). Snapshot deletion paths (snapshot remove, snapshot remove
--all, snapshot purge) auto-run CleanupOrphanedData inline so the
local index database doesn't accumulate ghost rows after every
removal — the user observed ~39k orphaned files and 2 orphaned blobs
after a remove --all because that cleanup was previously a separate
opt-in command. `snapshot prune` is removed.
Also addresses the doc/help-string drift the user audit caught:
* cli/prune.go help text used to reference a non-existent
`vaultik purge` command.
* cli/config.go get/set short/long examples were S3-specific
(s3.bucket) when the primary storage configuration is
storage_url.
* vaultik/info.go printed S3 Bucket/Endpoint/Region labels
unconditionally; for file:// or rclone:// users those rows
were empty. The Storage Configuration block now prints the
storer's Type+Location first, the storage_url string when set,
and only emits S3 rows that are actually populated.
* vaultik/info.go's "Run 'vaultik prune --remote'" hint
referenced a flag that doesn't exist.
* vaultik/blobcache.go's doc comment claimed LRU eviction, which
is no longer the restore-time policy (the sweeper drives
eviction; LRU is the safety-net fallback when maxBytes is
finite).
* README.md listed `vaultik restore`, `vaultik snapshot prune`,
and `s3.bucket` example, all out of date.
README's roadmap section is rewritten with concrete pre-1.0 items
(security audit, error-condition tests, parallel blob downloads,
restart of interrupted restore, …) so the next-steps surface
matches what the project actually still needs.
The cleanup calls are guarded against a nil SnapshotManager so
tests that construct a bare Vaultik struct continue to work.
Three coordinated changes drop restore wall-clock by orders of
magnitude on real-world snapshots and bring memory use back under
control:
* Streaming download into the disk cache. New
blobDiskCache.PutFromReader takes an io.Reader and io.Copy's it
straight into the cache file. The old downloadBlob path did
io.ReadAll on the decrypted plaintext stream — for a 50 GB blob
that meant 50 GB in RAM before the cache write. The whole chain
(Storage.Get → age.Decrypt → zstd.NewReader → io.Copy) is now
fully streaming; peak RAM per blob is bounded by ~64 KB of
internal age/zstd buffers plus the io.Copy buffer.
* Chunk extraction via ReadAt. After a blob is on disk, restore
reads chunks via blobDiskCache.ReadAt(hash, offset, length) so
only the chunk's bytes ever touch RAM. The previous code path
called blobCache.Get for every cache-hit chunk, which read the
entire blob (e.g. 10 GB) from disk into a []byte just to slice
out a few KB — single-handedly ~900 ms per cache hit on the
user's photo snapshot.
* Locality-aware restore ordering. New restorePlan indexes
file→blob_set and blob→file_set at restore start, then drives
the loop so that every file whose full blob set is on disk is
drained before any new blob downloads. After the drain queue
empties, the planner picks the pending file with the smallest
uncached-blob count, downloads those blobs, and drains again.
A sweep is forced right before each download so the just-
completed blob is evicted before the next one is Put, keeping
peak disk-cache occupancy at 1 for path-order-adversarial
snapshots.
The restore hot path also moves onto a restoreSession struct so
restoreFile/restoreRegularFile/etc. take only the per-call file
argument instead of threading 9+ parameters through every signature.
The new BlobRepository.GetAll method lets the session build a single
blob-id → blob-hash map at start instead of doing one DB query per
chunk.
TestRestoreLocalityAndReadAt passes: peak_len ≤ 1, get_calls = 0,
readat_calls > 0, every blob fetched exactly once.
Captures three behaviors the restore hot path must exhibit but
currently doesn't, all under one test:
* Peak blob disk cache occupancy ≤ 1. Smart restore ordering should
drain every file referencing the currently-cached blob before
downloading the next one, so the sweeper can free each blob the
moment its file set is exhausted.
* Every remote blob fetched exactly once (counter on a wrapping
Storer). Already true today; the test pins it so neither future
cache-eviction nor reorder regressions can introduce
re-downloads.
* blobDiskCache.Get is never called during restore — chunk
extraction must go through ReadAt so we never read the whole
blob from disk to slice out a few KB. The 10 GB
photo-snapshot --debug output showed ~900 ms per cache-hit chunk
extract; ReadAt should bring that to sub-millisecond.
Adds Get/ReadAt call counters and a peak-Len tracker to
blobDiskCache, plus an internal restoreCacheObserver hook on Vaultik
so the test can capture the production cache instance without
exporting unexported types.
Currently fails with peak_len=3, get_calls=24, readat_calls=0. The
fix follows in subsequent commits.
Restore previously capped the blob disk cache at 4× the configured
blob_size_limit (so 40 GB by default). With large or heavily-deduped
snapshots a chunk-by-chunk file walk could blow past that cap and
trigger LRU eviction of blobs that were still needed by later files,
forcing repeated re-downloads — observed during a real restore as
single-stream throughput collapsing to under 1 MB/s.
Restore now allocates the cache with no practical size cap and drives
eviction explicitly:
* An in-memory set of restored file IDs accumulates as files finish.
* Every blob_size_limit/100 bytes of restored data (≈100 sweeps per
blob's worth of writes) the sweeper iterates the cache. For each
cached blob it queries the snapshot's local SQLite DB for every
file that references any chunk in the blob and deletes the cache
entry only when every such file is already in the restored set.
* blobStillNeeded returns true on any error so an unreadable DB
never causes premature eviction.
The cache itself gains Delete(key) and Keys() so the sweeper can drive
removal without touching internal LRU state.
The disk blob cache now uses 4 * BlobSizeLimit from config instead of a
hardcoded 10 GiB default. This ensures the cache scales with the
configured blob size.
Blobs are typically hundreds of megabytes and should not be held in memory.
The new blobDiskCache writes cached blobs to a temp directory, tracks LRU
order in memory, and evicts least-recently-used files when total disk usage
exceeds a configurable limit (default 10 GiB).
Design:
- Blobs written to os.TempDir()/vaultik-blobcache-*/<hash>
- Doubly-linked list for O(1) LRU promotion/eviction
- ReadAt support for reading chunk slices without loading full blob
- Temp directory cleaned up on Close()
- Oversized entries (> maxBytes) silently skipped
Also adds blob_fetch_stub.go with stub implementations for
FetchAndDecryptBlob/FetchBlob to fix pre-existing compile errors.
clawbot
clawbot