amadan

carlos / platform Public

The replicator's sixth build: the fifth review's bounded findings

A file swap is held from the start of the last look that found every
file as it was, and a later finding widens what is held. What a fleet
passes on is the union of what it found and what it inherited; a fleet
closed before its predecessor does not latch an empty inheritance; Open
requires every fleet in the chain it replaces to be closed, and Open and
the one shutdown hold a lifecycle lock. The spelled directory path is
resolved one component at a time, following only the platform's links
and refusing any other, so a file nested beneath a platform-made state
link is owned by its resolved path and a tenant's link above its chain is
refused; the chain top's ancestry must equal its resolution; a
platform-owned start directory must be writable by nobody else. A sidecar
name is a sidecar only beside a database; a claimed or discovered file
that vanished before its final look is said. The tests drive the look,
hold a shutdown open, and table the witness retries. What stays open is
in the dispositions, review five.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude d63f81dfac97bdfd998d2e2bf722aa0d1efc1781 parent 1b3cc29
9 files changed, +960 −60
  • docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review5.md +188 −0
  • docs/superpowers/specs/DISPOSITIONS-metered-replicator.md +20 −0
  • docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review5.md +172 −0
  • internal/ops/litestream.go +6 −0
  • internal/ops/litestream_metered.go +128 −24
  • internal/ops/litestream_metered_test.go +252 −0
  • internal/ops/litestream_owners.go +104 −35
  • internal/ops/litestream_owners_test.go +10 −0
  • internal/ops/litestream_owners_unix_test.go +80 −1

CI passed — run details

diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review5.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review5.md
new file mode 100644
index 000000000..1512e3d0f
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review5.md
@@ -0,0 +1,188 @@
+All locations are at **`1b3cc29e`**. Both requested diffs were inspected. Nothing was modified; no tests or network commands were run. Statements about tests below are source-based counterfactuals, not executed mutation results.
+
+| Review-four finding | Closed? | Would its test pass without its fix? |
+|---|---|---|
+| **Opus P2-1:** DynamicUser state-directory link refused | **Partly.** A file immediately beneath the platform-made link works. A named file deeper beneath it still fails; finding 6. | **No** for the new immediate-directory-link fixture. Nested named files are untested. |
+| **Opus P2-2:** invalidation during admission’s witness read | **Closed for that interleaving.** The check at `internal/ops/litestream_metered.go:216` refuses it. This is not a fence against subsequently sending an already-admitted request. | **No.** The blocked-witness test would return success without the new check. |
+| **Opus P2-3:** header open follows a substituted FIFO/link | **Closed for the Unix final-component FIFO/link reproduction.** This does not secure directory traversal or bound all filesystem operations. | **No** for reverting the whole opening fix: the substituted FIFO blocks. Removing only `O_NOFOLLOW` is not independently tested: a different-inode target is also rejected by the identity check. |
+| **Opus P2-4:** relative/missing unit claims disappear | **Closed for claims already relative or missing at the first inspection.** Disappearance during final validation remains; finding 7. | **No** for removing either new reporting branch. The final-validation race has no test. |
+| **Opus P3-1:** upper-margin test is ineffective/missing | **Closed.** The advertised midnight case now exists. | **No.** Removing the upper margin omits October 3 in the new case. |
+| **Opus P3-2:** file hold starts at a registry proof | **Not closed.** The failing scan’s start is used, not the preceding successful scan’s start; finding 2. | **Yes.** `00:00:30 − 1 minute` already reaches the previous day in the new test. |
+| **Opus P3-3:** lineage comment promises a new instance lineage | **Closed as a documentation correction.** Same account/path deliberately reuses lineage. | No behavioral fix or corresponding test. |
+| **Opus P3-4:** an unopened replacement loses inherited uncertainty | **Partly.** Ordered passage through a replacement that never becomes stale works. Its own staleness masks the inherited window; early closure also loses it. | **No** for removing inheritance at Close or its fallback in `staleWindow`. **Yes** for removing the once guard: the tests do not assert exact application counts. |
+| **Opus P3-5:** standalone sidecar-shaped database ignored | **Partly.** A missing or symlink base no longer suppresses inspection. Any regular base still does; finding 8. | **No** for restoring unconditional suffix exclusion. A non-database regular base is untested. |
+| **Opus P3-6:** unit claim not bound to its host’s state root | **Documented, not fixed.** | No route-to-UID/root binding test. |
+| **Opus P3-7:** Close bypasses retries | **Closed in code.** Close uses `witnessUnchanged`; the final unnecessary delay is gone. | No recovered-error, exhausted-error or witnessed-change retry test. |
+| **Astra 1:** first-open substitution persists | **Open**, accurately disclosed in the current disposition. | No regression test. |
+| **Astra 2:** admission after filesystem invalidation | **Closed for invalidation during the witness read.** Already-granted admissions remain outstanding and untracked. | **No** for the new blocked-read case. |
+| **Astra 3:** affected dates escape holds | **Open.** The file boundary, delayed attempts and crash cases remain; finding 2. | **Yes** for removing the new file-boundary adjustment. The upper-margin test now detects its separate fix. |
+| **Astra 4:** shadow/claim ancestry unchecked | **Open**, now disclosed for both shadow and claim directories. | Existing tests still pass with these ancestors unchecked. |
+| **Astra 5:** another UID’s starting directory accepted | **Closed for the stated non-platform-UID reproduction.** The new starting-directory check rejects it. Authority is still inferred from the file, not the route. | **No.** Removing the starting UID check accepts the `shared/main.db` fixture. |
+| **Astra 6:** inventory omissions | **Partly.** Relative paths, initially missing unit claims, and several final-inspection failures are reported. Findings 7–8 remain. | **No** for the relative/missing/standalone-sidecar fixes. **Yes** for removing `discovered` reporting, case folding, arbitrary-extension header inspection, or header-error reporting: the relevant cases are absent. |
+| **Astra 7:** substituted file type hangs inspection | **Closed for the Unix FIFO/final-link case.** Broader traversal and unsupported-platform guarantees remain limited. | Whole-fix reversion fails the FIFO test; individual no-follow enforcement is not isolated. |
+| **Astra 8:** concurrent Close publishes completion early | **Closed for concurrent Close calls on the same fleet.** Other lifecycle paths still permit overlap; finding 5. | **Can pass.** The test waits for all callers and only checks the final state; it never blocks the first shutdown and checks an early caller/replacement. |
+| **Astra 9:** tests do not establish advertised properties | **Partly improved, still open.** Distinct UID and admission-read fixtures are useful; findings 2 and 9 identify remaining gaps. | **Yes** for several material mutations detailed below. |
+| **Astra 10:** shutdown’s transient-error policy differs | **Closed in code**, without focused coverage. | No corresponding error-injection test. |
+
+The review-four **claim corrections** are mixed. Both reviewers’ first-open-comment, missing-midnight-case and same-account-lineage objections are corrected. Astra’s settled-row crash wording is corrected. The distinct-tenant climb now has a seam-based fixture. The remaining claim contradictions are listed after the findings.
+
+The generation and lineage checks otherwise hold within their stated scope:
+
+- **C:** `internal/registry/registry.go:569` watches all six fields used by current discovery/attribution: `host`, `account`, `app`, `db_path`, `backing`, `hibernate`. The triggers at `:599–609` cover ordinary writers, including an older build’s raw SQL. Changed-value updates exclude unchanged values. The rowid trigger aborts the reported `UPDATE OR REPLACE` bypass. **“Nothing else” is only true structurally:** database-free inserts/deletes still advance the counter, as disclosed. The witness test still lacks a host-update case.
+- **E:** `internal/ops/litestream_metered.go:640–645` and `:664–665` use account plus cleaned path for both remote lineage and shadow state. Different valid account IDs do not share either key; renaming between accounts does not carry the old key. Same-account/path reuse is deliberate. Tests still do not exercise two successive accounts at one path or independently establish the remote-prefix change.
+- **B/I:** `witnessUnchanged` makes three total attempts, with two 200 ms delays and five-second contexts. Recovered errors do not invalidate; a witnessed change stops immediately; exhaustion invalidates conservatively. Approximately 15.4 seconds is the configured read-and-delay allowance, **not an end-to-end admission bound**: mutex waiting and filesystem inspection are outside that bound. Close skips the last reading when already stale.
+- The final stale load at `internal/ops/litestream_metered.go:216` can serve as the admission’s grant point. Invalidation can still occur between that load and `return nil`, and between admission and transport dispatch. The reported while-reading race is fixed; lifetime tracking is not.
+
+1. **P1 — Litestream can retain and operate on a substituted database indefinitely.**
+
+ **Locations:** `internal/ops/litestream_owners.go:268–280`; `internal/ops/litestream_metered.go:416–427`; `third_party/litestream/db.go:1004`, `:1014`, `:1866`.
+
+ Tenant A temporarily substitutes another database during Litestream’s initial SQLite/file opens, then restores the original pathname. Later inventories see the original inode and accept it, while Litestream retains the substituted handles. WAL access also continues by pathname.
+
+ The current disposition states this correctly. This is larger than a short interval between polling checks: the wrong database can remain open indefinitely, and the first unauthorized read or checkpoint can occur before any later identity comparison.
+
+ **Smallest acceptable fix:** preserve verified filesystem authority through SQLite, database and sibling opens using a pinned trusted root and constrained resolution. Verify the identities actually opened. An after-the-fact identity comparison alone cannot prevent the initial operation.
+
+2. **P1 — `goStaleFrom` still leaves affected days unheld.**
+
+ **Locations:** `internal/ops/litestream_metered.go:273–285`, `:410`, `:425–427`; `internal/lease/requestgate.go:58`.
+
+ The new argument is the start of the **scan finding the substitution**, not the last successful scan. Consider:
+
+ - A scan examines the database at October 2, 23:58, then continues walking a large tree.
+ - The tenant substitutes the database at 23:59; a request streams from it that night.
+ - That scan finishes after midnight. A registry admission proves its unrelated ownership counter unchanged at 00:02.
+ - The next scan begins at 00:02:10 and discovers the substitution.
+
+ Both candidate lower bounds are now after 00:02. Subtracting one minute still excludes October 2. This requires neither a crash nor a request delayed beyond the margin.
+
+ A second problem is `CompareAndSwap(false, true)` at `:274`: once one stale reason wins, a later finding cannot widen its window. A registry finding can therefore prevent a filesystem finding from adding an earlier affected date.
+
+ The separately disclosed delayed-attempt and crash cases also remain: admission does not track attempts through completion, and a completed request can survive a crash without a persisted ownership-transition hold.
+
+ **Smallest acceptable fix:** retain a separate last-successful filesystem proof boundary, initialized before construction’s inventory; conservatively combine uncertainty intervals instead of discarding later evidence. Track affected attempt dates through completion and persist transition uncertainty for the crash guarantee.
+
+3. **P1 — A replacement’s own stale window hides its inherited window.**
+
+ **Locations:** `internal/ops/litestream_metered.go:292–299`, `:313–317`, `:512–519`.
+
+ Fleet A closes with October 2 held. Fleet B is constructed on October 3 with `Replaces: A`, but never opened. Another ownership change occurs on October 4. B’s Close creates its own October 3–4 window, then inherits A’s October 2 window.
+
+ B’s recorders receive both. But `B.staleWindow()` returns immediately from the `f.stale` branch and discards the inherited interval. Fleet C replaces B and holds only October 3–4. **October 2 remains unheld on C’s recorders.**
+
+ This failure occurs with sequential calls and correctly ordered predecessor shutdowns. The new unopened-fleet test covers only a B that stays current.
+
+ **Smallest acceptable fix:** return the union of local and inherited uncertainty, preserving the earliest start and latest end—or an equivalent collection of intervals—and test an unopened B that itself becomes stale.
+
+4. **P1 — Platform-owned endpoints do not establish trusted ancestry; the new link exception also discards intermediate resolution authority.**
+
+ **Locations:** `internal/ops/litestream_owners.go:341–355`, `:388`, `:453–466`; `internal/ops/litestream_metered.go:687–710`.
+
+ A platform-owned `dataDir` underneath a tenant-writable ancestor passes the checks. The tenant can rename an ancestor and substitute another tree before Litestream resolves the metadata path.
+
+ The platform-link exception has a related concrete acceptance:
+
+ - A root-owned link in a protected public directory points to `/srv/A/choice`.
+ - `choice` is a tenant-controlled link currently resolving to `/protected/private/A`.
+ - `/protected/private` is root-owned; the resolved database and directory belong to A.
+
+ `EvalSymlinks` removes the tenant-controlled intermediate link from consideration. The final parent passes `platformOwned`, and the file is accepted under the original, still tenant-redirectable spelling.
+
+ A direct target whose immediate parent belongs to the distinct tenant is correctly refused. The hole is the unchecked ancestry/intermediate resolution, not that direct case.
+
+ For **A/J**, the ordinary inferred chain now rejects a stable link or another **non-platform** UID’s directory. Every accepting `tenantFile` path still reaches the top check at `:385`; earlier exits reject. A final-component `DBPath` symlink to another tenant’s file is rejected at `:321–323`.
+
+ A tenant sharing root/the process UID has no isolation under these rules. It can satisfy the platform-link exception and claim qualifying files outside its intended state directory. This is not literally acceptance “from anywhere”: local shape, permissions, link count and UID checks remain. Root-running units now receive that short local check rather than climbing indefinitely. Untrusted tenants must not share the platform UID.
+
+ **Smallest acceptable fix:** verify and pin the complete trusted hierarchy and permitted link resolution, rather than trusting only its endpoints. Bind isolated tenant claims to an independently trusted root/UID; treat same-UID services as explicitly trusted.
+
+5. **P2 — `closeOnce` does not prevent replacement overlap through an unopened intermediate fleet or concurrent Open.**
+
+ **Locations:** `internal/ops/litestream_metered.go:309–317`, `:345–371`, `:480–495`, `:519–520`.
+
+ Two remaining sequences:
+
+ - A is running. B is built with `Replaces: A`, then closed without opening. B does not require A to be closed before consuming its window and publishing `closed`. C replaces B and passes its Open check while A is still running or draining. If A only becomes stale later, its window has also already been missed.
+ - A’s Open copies `f.store`, then pauses before `store.Open`. Close drains that store and publishes completion. A replacement opens. The paused Open resumes and reopens the old databases. Litestream explicitly supports database reopening at `third_party/litestream/db.go:733–734`; the fleet has no lifecycle barrier preventing this.
+
+ The first sequence needs no concurrent calls. The concurrent-Close fix itself is sound: all Close callers wait for one shutdown and receive its result.
+
+ **Smallest acceptable fix:** serialize Open and Close with a lifecycle protocol separate from the admission mutex. Preserve unresolved predecessor shutdown dependencies through unopened replacements; consume their final windows only after those dependencies complete.
+
+6. **P2 — Named databases nested beneath a DynamicUser state link remain unowned.**
+
+ **Locations:** `internal/ops/litestream_owners.go:341`, `:375–388`.
+
+ With `/var/lib/carlos-app/A` as the platform-made link, a route naming `/var/lib/carlos-app/A/data/main.db` starts at the real-looking `data` directory. The climb encounters the link at `A`, stops, and calls `platformOwned(A)`, which rejects the link.
+
+ The fifth-build exception handles a link only when it is the file’s **immediate** directory. The new test covers only that shape.
+
+ **Smallest acceptable fix:** recognize and validate the authorized state-root link when the climb reaches it, then check the remaining path relative to its pinned target. Add a nested named-file fixture.
+
+7. **P2 — Discovered files still disappear silently on final `ENOENT`.**
+
+ **Locations:** `internal/ops/litestream.go:449–450`, `:488–504`.
+
+ The strict walk returns `/state/A/main.db`. Before final validation, the tenant removes or renames it. `discovered[p]` is true, but the reporting switch has no `ENOENT` case, so the candidate disappears from every result list. The tenant restores it after construction; the fleet starts without replicating or reporting it.
+
+ The same gap exists for an explicit unit file that exists at the initial inspection and vanishes before the final one. Initially missing unit claims are correctly reported.
+
+ **Smallest acceptable fix:** retain a skipped/problem entry for known unit claims and discovered candidates that vanish during final validation. Keep the intentional absent-console behavior separate.
+
+8. **P2 — A regular non-database base still hides a SQLite database with a sidecar suffix.**
+
+ **Locations:** `internal/ops/litestream_owners.go:536–545`, `:571–577`; `internal/ops/litestream_owners_test.go:511–522`.
+
+ Create a text file `archive` and a real SQLite database `archive-journal`. `sidecarOf` sees a regular base and suppresses the header read. Neither file is reported as a database. Uppercase companion suffixes are also suppressed, although on a case-sensitive filesystem they need not be SQLite’s actual companion name.
+
+ A symlink base is correctly rejected by `Lstat`; that narrower K case is fixed. But regular-file existence does not prove that the candidate is a sidecar—or that its base is a database.
+
+ The new test even writes a **SQLite database header** into `live.db-wal` and requires it to be ignored. That is not a valid demonstration of a real WAL file.
+
+ **Smallest acceptable fix:** inspect for a SQLite database header before applying the sidecar exclusion. A database header must be reported regardless of a neighboring regular file.
+
+9. **P2 — Important regression tests still pass without the behavior they claim to establish.**
+
+ **Locations:** `internal/ops/litestream_metered_test.go:687`, `:884–889`, `:943–973`, `:1088–1098`, `:1154–1184`; `internal/ops/litestream_owners_test.go:357`.
+
+ Concrete surviving mutations:
+
+ - Ignore `lookStart` entirely: the new file-swap test still holds October 2 because its registry proof is only thirty seconds after midnight.
+ - Remove `store.Close`: nulling the fleet’s store and closing its gate still satisfy the shutdown assertions. Retained database handles and worker termination are not checked.
+ - Remove `closeOnce`: the new test can still finish with three nil results and the expected final state. It does not observe completion while one shutdown is deliberately blocked.
+ - Remove the watch’s filesystem-check call: the filesystem test invokes `lookAgain` directly.
+ - Remove case folding, arbitrary-extension inspection or header-error reporting: the strict-walk fixture does not isolate those cases.
+ - Misattribute some store-driven requests: the opened test checks positive counts for both databases and server inactivity, not settled received-versus-recorded attempts per database.
+
+ The former distant-temp-root climb dependence is removed for these fixtures, and the UID seam improves coverage. Remaining portability limits include hard-coded synthetic UIDs `44001/44002` without excluding the actual process UID, and unreadable-directory tests skipped under root.
+
+ Timestamp-table cases are deterministic. The integration checks still use fixed sleeps, a 500 ms quiet period and a 15-second shutdown threshold; they do not establish a scheduling-independent completion barrier.
+
+ **Smallest acceptable fix:** choose timestamps that distinguish the competing lower bounds; synchronize shutdown/admission interleavings; assert retained handles and workers are closed; compare settled attempts by database; exercise filesystem invalidation through the watch; add the missing classification/error fixtures.
+
+The remaining requested scope checks do not change those findings. Case-insensitive SQLite names are recognized, but uppercase `.DB` is **reported as skipped**, not replicated. Unreadable directories are problems; no-path unit routes are problems; explicit empty unit files are skipped. Empty tree entries, selected vanished entries and exec paths outside the globs remain excluded. No external-replicator inventory comparison establishes cutover completeness.
+
+On Unix, `openRegularNoFollow` checks the opened descriptor’s type, device and inode before reading, and correctly handles the demonstrated final-component FIFO/link race. `O_NOFOLLOW` applies only to the final component. It does not secure parent traversal, and `O_NONBLOCK` is not a universal bound on filesystem/device operations. On non-Unix builds, `internal/ops/sqliteopen_other.go:9` is an ordinary unchecked open. Although ownership classification subsequently fails through `filefacts_other.go:16`, inventory runs first; unsupported-platform safety should not be inferred from that later refusal.
+
+**Claims the code contradicts**
+
+| Claim raised in review four | Status at this commit |
+|---|---|
+| **Opus/Astra:** the first-open exception was absent from the ownership comment | **Corrected** at `internal/ops/litestream_owners.go:268–280`. The older disposition at `docs/superpowers/specs/DISPOSITIONS-metered-replicator.md:84` still offers exposing identity **or** constrained opening; identity detection alone is insufficient prevention. |
+| **Opus/Astra:** the advertised midnight case did not exist | **Corrected** at `internal/ops/litestream_metered_test.go:491`. |
+| **Opus:** the stale re-check covered invalidation during the read | **Now true for that interleaving**, at `internal/ops/litestream_metered.go:216`. It does not establish a transport fence. |
+| **Opus:** `ErrOwnershipStale` promised “every replica request is refused from then on” | **Still present** at `internal/ops/litestream_metered.go:33`. The narrower replacement wording was appended at `:36–39`, rather than replacing the old claim. |
+| **Opus/Astra:** the entire tenant chain belongs to the tenant | **Still too broad** at `internal/ops/litestream_owners.go:237–241`. The starting directory may belong to the platform, and the new link exception can conceal intermediate tenant-controlled resolution. The reported non-platform starting-UID bug itself is fixed. |
+| **Opus/Astra:** the hold covers the doubtful dates and uses the last successful file look | **Still false.** `internal/ops/litestream_metered.go:259–262` describes a saved successful look; `:410` actually captures the current failing scan. `:550` still promises all relevant dates are held. |
+| **Opus/Astra:** a crash leaves only started rows | **Corrected** at `internal/ops/litestream_metered.go:552–558`; settled rows without a hold are explicitly acknowledged. |
+| **Opus/Astra:** deleting/recreating an instance creates a new lineage | **Corrected** at `internal/ops/litestream_metered.go:617–623`; same-account/path reuse is explicit. |
+| **Opus/Astra:** strict discovery passes nothing over silently / checks every other regular file | **Still too broad**, at `internal/ops/litestream_owners.go:479` and dispositions `:87`, `:116–117`. Findings 7–8 remain, besides the explicitly documented empty/churn exclusions. |
+| **Opus:** the distinct-tenant CI claim was untested | **Improved:** the seam now exercises a distinct-owner climb. The claimed `/tmp` and cache-root test results at dispositions `:130` were not independently verified in this source-only review. |
+| **Astra:** a missing pathname means nothing is being shipped | **Still false** at `internal/ops/litestream_metered.go:422`. Retained descriptors and pending shadow files can outlive the pathname. |
+| **Astra:** only ownership moves the generation | **Still semantically too broad** at `internal/registry/registry.go:580–581`; database-free insertion/deletion also moves it. The disposition accurately acknowledges that behavior. |
+| **Astra:** the migration adds three triggers | **Still stale** at dispositions `:75`; there are four. |
+| **Astra:** only an operator or systemd can make an undetected bind mount | **Still overstated** at dispositions `:95`. The prerequisite is sufficient mount authority in a namespace visible to the replicator, not a particular actor’s identity. Same-device checks do not establish its absence. |
+| Fifth-build disposition: unopened replacements preserve inherited windows | **False without qualification**, at dispositions `:122`; findings 3 and 5 give sequential counterexamples. |
+| Fifth-build comment: a replacement can never open beside a draining store | **Too broad**, at `internal/ops/litestream_metered.go:468`; `closeOnce` fixes the duplicate-Close path, not the lifecycle paths in finding 5. |
+
+**Verdict: reject.**
\ No newline at end of file
diff --git a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md
index 82feabc0d..60953d96b 100644
--- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md
+++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md
@@ -128,3 +128,23 @@ Reports: `OPUS-REVIEW-metered-replicator-review4.md`, `CODEX-FINDINGS-metered-re
| Astra 9 (P2): tests that do not establish several claims (retained handles closed, per-database attribution, the watch invoking the look, fixed sleeps). | Open in part: the owner-chain tests now run with distinct owners; the admission race has a deterministic case; the rest (handles, attribution, the watch's own invocation) are as stated. | | |
Package tests pass under `/tmp` and under `~/.cache` (the CI failure of the fourth build was the lesson). The full gate on this tip is the next step; the fifth build has not been read by a reviewer.
+
+## Review five (reviewed `1b3cc29e`; Opus accepted with fixes, Astra rejected)
+
+Reports: `OPUS-REVIEW-metered-replicator-review5.md`, `CODEX-FINDINGS-metered-replicator-review5.md`. Both found the fifth build's bounded fixes present; both found the file-swap lower bound wrong (the look that found the swap, not the last clean look) and the inherited window lost in two orderings; Astra's P1s on litestream's own opens and shadow ancestry stand as before. This section records the sixth build, which is unreviewed.
+
+| Finding | Disposition | Where | Reproduction |
+| --- | --- | --- | --- |
+| Opus P2-1, Astra 2 (P1): the hold for a file swap ran from the start of the look that found it, not from the last look that found every file as it was; a second finding could not widen the first's window. | Fixed: `lastClean` is the start of the last look that found nothing (seeded by the constructor's inventory); a swap is held from before it; a later finding widens the window and holds only the days it adds. Not fixed: requests tracked to completion across midnight, and the crash case, as stated before. | `litestream_metered.go` `lookAgain`, `goStaleFrom` | `TestAFileSwapIsHeldFromTheLastCleanLook` (driven through `lookAgain`, with the registry proof an hour old and the last clean look two days old: the clean look's day is held; a finding within the window adds nothing; one further back widens) |
+| Astra 3 (P1), Opus P3-1 in part: a replacement that went stale after inheriting passed on its own window alone; a fleet closed before its predecessor latched an empty inheritance. | Fixed: `staleWindow` is the union of own and inherited; `inheritHold` latches only once the predecessor has closed, and an unlatched fleet answers for its predecessor through `staleWindow`. | `litestream_metered.go` | `TestInheritedWindowIsTheUnion` (B stale after inheriting A: C holds both days; B closed before A: C holds A's later day through B) |
+| Opus P3-1, Astra 5 (P2): `Open` checked one predecessor; `Open` racing `Close` could open a store already torn down. | Fixed: `Open` requires every fleet in the chain it replaces to be closed; `Open` and the one shutdown hold a lifecycle lock, and `Open` after `Close` is `ErrOwnershipStale`. | `litestream_metered.go` | `TestOpenRefusesWhileAnyPredecessorIsOpen`, `TestCloseBlocksEveryCallerUntilTheStoreHasStopped` (a shutdown held open by a hook: a second caller does not return, a replacement cannot open, the fleet does not read as closed, until it finishes) |
+| Opus P3-3, Astra 4 (P1) in part: a link inside the spelled path above the chain's top was followed, so another tenant's tree was reached under a tenant's route; the platform-made start link resolved through whatever its target passed through. | Fixed: the spelled directory path is resolved one component at a time, following only the platform's links (the link and its parent the platform's, an absolute target with no further link, the target's parent the platform's) and refusing any other; the chain top's ancestry must then equal its resolution. A file nested at any depth beneath a platform-made link is owned by its resolved path (Astra 6). Not fixed: `dataDir`'s own ancestry and the shadow directories (Astra 4): `platformOwned` still checks a directory, not its ancestry; a verified immutable hierarchy is approval H's question. | `litestream_owners.go` `resolvePlatformLinks` | `TestUnitFileChainRefusesALinkAboveIt`, `TestUnitFileThroughAPlatformMadeStateLink` (nested file; a platform link whose target passes through the tenant's link, refused) |
+| Opus P3-4: a platform-owned start directory was not checked for world-writability; a tenant whose uid is the platform's has no isolation. | Fixed for the first: a platform-owned start directory must be writable by nobody else. The second is stated: an untrusted tenant must not share the platform's uid (operator rule, with approval H). | `litestream_owners.go` | `TestUnitFileInAWorldWritablePlatformDirectory` |
+| Opus P3-6, Astra 8 (P2): a database under a sidecar name beside an unrelated regular file was never read. | Fixed: a sidecar name is a sidecar only beside a database (by name or by header). | `litestream_owners.go` `sidecarOf` | `TestStrictTreeReadsAStandaloneSidecarName` |
+| Opus P3-6, Astra 7 (P2): a claimed or discovered file that vanished before its final look disappeared. | Fixed: it is listed as skipped. | `litestream.go` | no deterministic test (the race between the walk and the final look is not simulated); stated |
+| Opus P3-5: the header read's `O_NOFOLLOW` covers the last component only; a directory swapped higher up can lead the open to a device; no `O_NOCTTY`. | Open: a walk by directory descriptor (`openat`, `openat2` with `RESOLVE_NO_SYMLINKS`) is the fix and is not built; `O_NOCTTY` is not added either, since the open is still by path. Stated. | | |
+| Opus P3-7, Astra 9 (P2): tests that passed without their fixes (the swap's lower bound; the close-once; no retry tests). | Fixed: the swap test drives `lookAgain` with bounds that tell the two apart; the close test holds the shutdown open; the witness retries have a table (recovered error, exhausted errors, a change, a change after an error). Still open: retained database handles and worker termination are not asserted; attribution per database is not compared; the watch's own invocation of the look is not exercised. | tests | as named |
+| Astra 1 (P1): litestream retains a substituted database opened by path. | Open, as before; needs a litestream change. Nothing is attested until it is. | | |
+| Astra claims (dispositions line about dropping the old name first; `ErrOwnershipStale` "from then on"; the test fixtures' umask and base-name seam). | The order is CREATE then DROP, stated; the fixtures depend on umask 022 (stated here); the seam keys on base names, safe for these fixtures and said so. | this file | |
+
+Package tests pass under `/tmp` and under `~/.cache`. The full gate on this tip is next; the sixth build has not been read by a reviewer.
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review5.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review5.md
new file mode 100644
index 000000000..3555f099e
--- /dev/null
+++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review5.md
@@ -0,0 +1,172 @@
+# Fifth review: metered replicator at `1b3cc29e` (Opus)
+
+**Verdict: accept with fixes.**
+- No P1 found.
+- One P2: the fix for the file-swap hold's lower bound (Opus P3-2) uses the wrong instant, and its test passes without the fix.
+- The rest are P3s, plus claims the code contradicts.
+- The code is unwired. Astra 1 (the swap during litestream's first open) and Astra 4 (unchecked ancestry) still block any attestation.
+
+Line numbers are at `1b3cc29e`. I read everything with `git show`. Nothing was run or modified.
+
+## Review four, finding by finding
+
+| Finding | Now | Would its test pass without the fix? |
+|---|---|---|
+| Opus P2-1 (DynamicUser link) | **Closed** (`litestream_owners.go:341-359`). Caveat in J2: the components of the link's target are not checked. | **No.** Without the fix the start link fails at `:364`. |
+| Opus P2-2 / Astra 2 (admission during the witness read) | **Closed for the read window** (`litestream_metered.go:216`). A request admitted before the change and sent after it is inherent, and is stated. | **No** (`TestAdmissionIsRefused…`, `_test.go:1049`). |
+| Opus P2-3 / Astra 7 (FIFO or link at the header read) | **Closed for the last path component.** Two things remain: a directory swapped for a link higher up the path (K), and a walk that cannot be cancelled. | **No** for the FIFO and the link. The device-and-inode check alone is never exercised: no test swaps one regular file for another. |
+| Opus P2-4 / Astra 6 (silent drops) | **Mostly closed.** A relative path and a missing claim are now reported. Two drops remain (L). | **No** for the relative and missing cases. **Yes** for `discovered[p]` (`litestream.go:494`): no test has a discovered file that fails its final look. |
+| Opus P3-1 (top of the hold) | **Closed.** | **No.** The new "found just before midnight" case fails with `to = at`. |
+| Opus P3-2 / Astra 3 (lower bound of a file-swap hold) | **Not closed** (P2-1 below). | **Yes** (P2-1). |
+| Opus P3-3 (lineage comment) | **Closed** (comment, `:617-623`). | n/a |
+| Opus P3-4 (window passed on transitively) | **Closed** for build A, stale, close A, build B, close B, build C. The order where B closes before A does is not handled (P3-1 below). | **No.** |
+| Opus P3-5 / Astra 6 (sidecar names) | **Partly closed.** The base file must exist, but it need not be a database (L). | **No.** |
+| Opus P3-6 (unit `DBPath` not tied to its host's state directory) | **Open, stated.** | n/a |
+| Opus P3-7 / Astra 10 (retry at `Close`) | **Closed in code** (`:233-250`, `:512-515`). | **No test at all.** Nothing sets `witness` to return an error. The dispositions' reproduction, "the admission tests", does not exist. |
+| Opus claims | First-open comment, crash wording and lineage comment: corrected. The CI claim is now tested through the `factsOf` seam. Two remain: the old `ErrOwnershipStale` paragraph (`:33`) and the "passes nothing over" claim. See Claims. | |
+| Astra 1 (first-open swap), Astra 4 (shadow ancestry) | **Open, stated.** See G for how they are sized. | n/a |
+| Astra 5 (start directory's owner) | **Closed** (`:364-369`). Caveats in J1. | **No** for the `shared` case. **Yes** for the `theirs.db` case (`unix_test.go:65`): without the start check it still fails at `platformOwned(A)`, which gives the same "belongs to a user other than" message. |
+| Astra 8 (concurrent `Close`) | **Closed** (`:469-472`, and `closed` is set at `:520`). An `Open`/`Close` race remains (P3-2 below). | **Yes** (H). |
+| Astra 9 (tests that do not establish their claims) | **Open in part, as stated.** | |
+| Astra claims | Two stand: "three triggers" (dispositions `:75`, there are four) and "only an operator or systemd" can make a bind mount (`:95`). The generation wording is now accurate. | |
+
+## Findings
+
+**P2-1. A file-swap hold starts before the look that found the swap, not before the last look that found the file unchanged. A day in doubt can still go unheld (M).**
+- **Where:** `litestream_metered.go:410` (`began := time.Now()`) and `:427` (`goStaleFrom(began, …)`). The comment at `:259-262` says "the start of the last look at them that found nothing".
+- **Scenario:**
+ 1. A tenant fills its tree with many files, so each walk takes about two minutes (every regular file is now opened for its header).
+ 2. Look N begins at 23:58:00 and checks `x.db` at 23:59:45.
+ 3. The tenant swaps `x.db` at 23:59:50. The fleet streams the swapped file and records it on day D, to the tenant.
+ 4. Look N ends at about 00:00:00. The ticker has a tick pending, so look N+1 begins at about 00:00:00 or later (`began`).
+ 5. The 1 s watch keeps `proven` later than `began`, so `sound = began` and `from` is about 23:59:00 or later (at most about 23:59:00).
+ 6. If look N+1 began at 00:01:00 instead (a slower walk N), `from` is 00:00:00 and day D is unheld while it holds a settled row for another file's bytes.
+- **Test:** `TestAFileSwapIsHeldFromBeforeTheLookBegan` (`_test.go:1088`) passes without the fix. With `lookStart` ignored, `from = proven − 1 min = 23:59:30` on 2 October, so both days are held anyway. It also calls `goStaleFrom` directly, so it never checks what `lookAgain` passes.
+- **Smallest fix:**
+ - Store the start time of the last look that completed and found every file unchanged. Seed it with the start of the constructor's inventory.
+ - Pass that value, not `began`, to `goStaleFrom`.
+ - In the test, set `proven` so that `proven − margin` falls after midnight, then drive `lookAgain` itself.
+
+**P3-1. `Replaces` is checked one level deep, and a never-opened fleet locks in its predecessor's window at its own `Close` (D, I, M).**
+- **Where:** `:346` (`Open` checks only `f.replaces.closed`); `:519` and `:309-319` (`inheritHold` at `Close` does not require the predecessor to be closed).
+- **Scenario:**
+ 1. A is open.
+ 2. B is built with `Replaces: A` and closed without opening. Its `inheritHold` reads A's window while A is still current, finds nothing, and the CAS locks that in.
+ 3. A goes stale and holds the old owner's days.
+ 4. C is built with `Replaces: B`. `C.Open` passes because B is closed, and inherits nothing.
+ 5. The new owner's recorder in C is never told. If C opens before A closes, two stores run over the same shadow directory and prefix.
+- **Smallest fix:** in `inheritHold`, do not latch until `replaces.closed`. Have `staleWindow` of an unlatched fleet delegate to `replaces.staleWindow()`. Have `Open` walk the `replaces` chain and require every link to be closed.
+
+**P3-2. `Open` racing `Close` on the same fleet can open a store that has already been torn down (I).**
+- **Where:** `:359-367` against `:480-497`.
+- **Scenario:**
+ 1. `Open` passes `admit`, copies `f.store`, and releases the lock.
+ 2. `Close` takes the store, closes it (unopened, so this is quick), sets `closed` and releases the witness.
+ 3. `Open` then calls `store.Open`, which opens each database (including litestream's read transaction) on a fleet that reads as closed.
+ 4. The gate refuses uploads, but a replacement can open beside this store's local shadow work.
+- **Smallest fix:** a lifecycle mutex held across `store.Open` and across `closeNow`. Or, after `store.Open`, re-check `closed` and close the store if it is set.
+
+**P3-3. The chain checks trust `Lstat` on ancestors built from the spelled path, so a link inside the spelled path above the chain top is followed rather than refused (J).**
+- **Where:** `litestream_owners.go:370-390`. `filepath.Dir` is lexical, and `Lstat`/`platformOwned` do not follow only the last component; links earlier in the path are followed.
+- **Scenario:**
+ 1. The route is `DBPath=/var/lib/carlos-app/T/a/carlos-app/V/main.db`, and T makes `a → /var/lib/private`.
+ 2. `root` resolves to V's directory, so the start directory's owner and the file's owner are both V.
+ 3. The climb's parent, `…/T/a/carlos-app`, resolves to a root-owned directory, so the climb breaks there.
+ 4. `platformOwned(…/T/a/carlos-app)` passes, and the downward walk sees only `main.db`.
+ 5. V's file is accepted under T's route.
+- **When it is refused:** only when V's file is also listed, because `SameFile` makes both unowned (fail closed).
+- **When it is not:** when V's file is unlisted, for example the state directory left behind by a deleted route.
+- **Related:** a platform-made start link (`:348-355`) checks only `Dir(real)`. A link whose target passes through a component the tenant controls is resolved wherever the tenant points it.
+- **Why P3:** both need an operator-chosen deep `DBPath` or an operator-made link.
+- **Smallest fix:**
+ - Require `EvalSymlinks(root) == root` after the start-link substitution.
+ - For the start link, require its `Readlink` target to be absolute and equal to its own `EvalSymlinks` result.
+
+**P3-4. A start directory that is the platform's is never checked for write permission, and a tenant whose uid is the platform's is accepted almost anywhere (A, J).**
+- **Where:** `:367` (`sf.UID == 0 || euid` passes with no mode check).
+- **Scenario:** a root-owned unit route file, `/var/tmp/x.db`:
+ - `/var/tmp` is root's, mode 1777, so it passes the start check.
+ - `/var` is root's, so the climb stops at once and `platformOwned(/var)` passes.
+ - The file is accepted. Any local user can then plant `x.db-wal` as a link, which SQLite or litestream opens as root.
+- **On a root-run box:** the climb never climbs; one directory and its parent are all that is checked (operator trust, as in review four).
+- **Is the top-of-chain link check still reached?** Yes, on every path out of the loop (`:385`).
+- **Smallest fix:** apply the `0o022` check to the start directory whenever its owner is the platform's. Refuse an owner uid that is the platform's for `tenantFile`, unless the route is the platform's.
+
+**P3-5. A swap of a directory higher up the path can make the header open hit a device (K).**
+- **Where:** `sqliteopen_unix.go:18`. `O_NOFOLLOW` covers only the last component, and there is no `O_NOCTTY`.
+- **Scenario:**
+ 1. The walk `Lstat`s `T/d/watchdog` as a regular file.
+ 2. T renames `d` and puts `d → /dev` in its place.
+ 3. Root opens `/dev/watchdog` (non-blocking), arming the watchdog on drivers that arm on open, before `fstat` refuses the file.
+- **Also:** `os.ReadDir` follows a directory swapped the same way. The results fail closed in `checkOwnedFile`, but the walk reads another tenant's file names and headers as root.
+- **Platforms without `O_NOFOLLOW`:** none. The `!unix` build never owns anything, because `factsOf` returns false there.
+- **A sidecar whose base is a link:** handled; it is read as an ordinary file.
+- **Smallest fix:** walk by directory descriptor (`openat` with `O_NOFOLLOW|O_DIRECTORY`, then `fstatat`/`openat` relative to it, or `openat2` with `RESOLVE_NO_SYMLINKS` on Linux), and add `O_NOCTTY`.
+
+**P3-6. Two silent drops remain (L).**
+- **A database under a sidecar name beside an unrelated regular file.**
+ - Where: `sidecarOf`, `:571-582`.
+ - Scenario: `notes.txt` (text) and `notes.txt-journal` (a SQLite database). The second is never header-read and never reported.
+ - Fix: require the base to be a database the walk found (`*.db`, `sqliteName`, or one with SQLite's header).
+- **A claimed or discovered file that vanishes before its final look.**
+ - Where: `litestream.go:494-503`. The switch has no `ENOENT` case.
+ - Probably acceptable as churn, but the "files the walk found count as claimed" fix does not report the vanished case.
+- **Still dropped as disclosed:** exec paths outside the globs, empty files in a tree, and entries that vanish during the walk.
+
+**P3-7. Tests (H).**
+- `TestCloseRunsOnceForEveryCaller` (`:1154`) passes without `closeOnce`. It checks state only after every caller has returned, which the old code also satisfied. It needs a store whose `Close` blocks, so the test can assert that a second caller does not return, and a replacement cannot `Open`, until the first finishes.
+- `TestAFileSwap…` passes without its fix (P2-1).
+- The witness retry has no test: no case for a recovered error, exhausted errors, or a witnessed `false`.
+- `discovered[p]` is untested.
+- The `theirs.db` case is redundant (see the table).
+- The device-and-inode branch is untested.
+- Environment: fixtures rely on `MkdirAll(0o755)` coming out exactly 0755. Under umask 002, `apps` and `carlos-app` become 0775 and `platformOwned` fails. That is a dependence on umask, not on the temp root.
+- The `factsOf` seam keys owners by base name only. That is safe for these fixtures because the climb stops at `apps` and `private`, but a temp path with a component named `A`, `data` or `private` above that point would be overridden too.
+- Wall clock: the new tests use fixed times or assert only non-zero counts, so they do not depend on it.
+
+## Answers to A–M not covered above
+
+- **A.**
+ - A tenant with its own user: no link or other user's directory can get a file accepted within the chain the code examines. The gap is the ancestry above it (P3-3).
+ - A tenant whose user is the platform's, and a box whose units run as root: see P3-4.
+- **B.**
+ - A request admitted before a change still reaches the network after it (inherent, stated).
+ - The re-check under the lock is now complete for the read window.
+ - The retries are bounded at 3 × 5 s + 2 × 200 ms = 15.4 s per admission, held under `f.mu`.
+ - Only an error is retried. A witnessed `false`, including a replaced registry file, is final.
+ - Two small gaps: a request whose context expires while it waits for the mutex is not re-checked after it gets the lock, and nothing tests the retries.
+- **C.** Closed, unchanged since review four. Inserts, deletes and changed-value updates all move the generation through triggers. `INSERT OR REPLACE` and `UPDATE OR REPLACE` on `host` move it too, and the rowid trigger holds.
+- **D.**
+ - No reading is taken on a stale fleet (`:512`).
+ - The CAS applies the inherited window once.
+ - A window can still be lost between fleets in the P3-1 ordering.
+- **E.** Unchanged and correct as now stated. Different accounts get different lineages and shadow state. Within one account the path is the lineage. A path that moves to another account starts a new key.
+- **F.** As intended:
+ - Upper-case names are reported as skipped.
+ - Every regular file is header-read except `*.db`, `sqliteName` files and sidecars that pass `sidecarOf` (P3-6).
+ - An unreadable directory is a problem. A missing root is a problem from the walk and skipped from the route loop.
+ - A unit route with no path is a problem. An empty claimed file is skipped.
+- **G.**
+ - **First-open swap:** stated correctly in code and dispositions, but slightly larger than stated. Any later re-initialisation by litestream reopens the database by path, not only the first open.
+ - **Ancestors of `dataDir`:** larger than stated. P3-3 shows that the spelled path's own ancestry is trusted, not only the ancestry of a platform directory.
+ - **Crash gap:** now stated correctly (`:551-558`).
+ - **Bind mounts:** stated, but Astra's wording point stands.
+ - **External inventory:** stated correctly.
+ - **Missing from the list:** P2-1, which is marked fixed but is not.
+- **I.** See the `admit` row in the table, and P3-1 and P3-2.
+- **J.** See P3-3 and P3-4. A route whose `DBPath` is itself a link to another tenant's file is refused at `:322`. A link at the directory level made by the tenant is refused at `:342`.
+- **K, L, M.** See P3-5, P3-6, P2-1 and P3-1.
+
+## Claims the code contradicts
+
+- **`ErrOwnershipStale` (`litestream_metered.go:28-35`):** the old paragraph, "Every replica request is refused from then on", is still there above the new one, and godoc merges the two. The dispositions say it was corrected.
+- **`goStale`'s doc (`:259-262`):** says the lower bound is "the last look at them that found nothing". The code uses the current look (P2-1).
+- **Dispositions, Opus P3-2 row:** "a swap found by the second look is held from before that look began" describes the code, but that is not the fix review four asked for, and its reproduction passes without it.
+- **Dispositions, Opus P3-7 row:** the reproduction, "the admission tests", does not exist.
+- **Dispositions, Astra 8 row:** `TestCloseRunsOnceForEveryCaller` does not establish the claim.
+- **`Close`'s first doc paragraph (`:460-464`):** "Closing a fleet that was current holds nobody's day". The last reading can `goStale` after its retries are exhausted.
+- **`strictTreeDBs` doc (`:479`, `:484-485`):** "passes nothing over in silence" and "a file with no extension that begins with SQLite's header". It now reads every file, and still drops the P3-6 case.
+- **`TestUnitFileThroughAPlatformMadeStateLink` (`unix_test.go:112` against `:120`):** the comment says "reports the file by its resolved path", but the assertion checks the spelled path.
+- **Dispositions `:75`:** "three triggers". There are four.
+
+**Verdict: accept with fixes.** Before merging, fix P2-1 (track the last clean look's start and give it a test that fails without the fix), fix P3-1 and P3-2 (lifecycle), and correct the claims above. Fix P3-3 and P3-4 before any attestation, alongside Astra 1 and 4.
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go
index 03fc23633..88386b633 100644
--- a/internal/ops/litestream.go
+++ b/internal/ops/litestream.go
@@ -495,6 +495,12 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t
switch {
case err != nil && !errors.Is(err, fs.ErrNotExist):
problems = append(problems, p+": cannot be looked at")
+ case err != nil && (discovered[p] || unitPaths[p] != ""):
+ // Found a moment ago and gone now: a tenant renaming a
+ // database under the look. Said, so a database that
+ // comes back after the build is known to have been
+ // unreplicated until the next look.
+ skipped = append(skipped, p)
case err == nil && !fi.Mode().IsRegular():
problems = append(problems, p+": not a regular file")
case err == nil:
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go
index e91755060..fd0a00331 100644
--- a/internal/ops/litestream_metered.go
+++ b/internal/ops/litestream_metered.go
@@ -146,6 +146,18 @@ type MeteredFleet struct {
closed atomic.Bool
inherited atomic.Bool
proven atomic.Int64 // the last instant ownership was shown unchanged, in Unix nanoseconds
+ // lastClean is the start of the last look at the files that found
+ // every one of them as it was (the constructor's inventory to begin
+ // with), in Unix nanoseconds: the instant before which no file is in
+ // doubt when a later look finds one changed.
+ lastClean atomic.Int64
+ // life serialises Open and Close with each other: a store is never
+ // opened on a fleet that is closing or closed, and never closed under
+ // an Open in progress.
+ life sync.Mutex
+ // closingHook, when set by a test, runs inside the one shutdown after
+ // the store is taken and before it is closed.
+ closingHook func()
staleMu sync.Mutex
staleFrom, staleAt time.Time
@@ -256,10 +268,11 @@ func (f *MeteredFleet) witnessUnchanged() (ok bool, err error) {
// The days in doubt run from a margin before the last instant the fleet
// was shown sound to a margin after the instant the change was found. For
// a change in the registry that instant is the last proof (the witness).
-// For a change in the files it is the start of the last look at them
-// that found nothing (lookStart): a registry proof says nothing about when
-// a file changed, so a swap found by a slow look is held from before that
-// look began. A request sent before the change was sent for the owner of
+// For a change in the files it is the start of the last look at them that
+// found every file as it was (lastClean): a registry proof says nothing
+// about when a file changed, and the look that finds a swap says nothing
+// about how long before it the swap was made, so the hold runs from
+// before the last look that found nothing. A request sent before the change was sent for the owner of
// that moment and is counted to them. The ones in doubt were admitted
// before the change and recorded after it; they were admitted no earlier
// than a moment before the change, which is later than that instant less
@@ -270,44 +283,114 @@ func (f *MeteredFleet) goStale(at time.Time) {
f.goStaleFrom(time.Time{}, at)
}
+// goStaleFrom makes the fleet stale (or, if it is stale already, widens
+// what it holds: a second finding, by the files after the registry or the
+// other way round, can reach further back than the first, and must not be
+// lost to the first). lookStart, when set, is the start of the last look
+// at the files that found nothing; the hold runs from the earlier of that
+// and the last registry proof, less the margin.
func (f *MeteredFleet) goStaleFrom(lookStart, at time.Time) {
- if !f.stale.CompareAndSwap(false, true) {
- return
- }
sound := time.Unix(0, f.proven.Load())
if !lookStart.IsZero() && lookStart.Before(sound) {
sound = lookStart
}
from, to := sound.Add(-ownershipMargin), at.Add(ownershipMargin)
f.staleMu.Lock()
+ if f.stale.Load() {
+ // Already stale: hold only the days the wider window adds, and
+ // tell the recorders nothing about days already held.
+ oldFrom, oldTo := f.staleFrom, f.staleAt
+ if !from.Before(oldFrom) && !to.After(oldTo) {
+ f.staleMu.Unlock()
+ return
+ }
+ from, to = earlier(from, oldFrom), later(to, oldTo)
+ f.staleFrom, f.staleAt = from, to
+ f.staleMu.Unlock()
+ if from.Before(oldFrom) {
+ holdDaysBefore(f.recorders, from, oldFrom)
+ }
+ if to.After(oldTo) {
+ holdDaysAfter(f.recorders, oldTo, to)
+ }
+ return
+ }
f.staleFrom, f.staleAt = from, to
+ f.stale.Store(true)
f.staleMu.Unlock()
holdDays(f.recorders, from, to)
}
-// staleWindow is the span a stale fleet held, for the fleet that replaces
-// it; a fleet that never went stale itself passes on the window it
-// inherited from the one it replaced, so a fleet closed without ever
-// opening does not drop it.
+// holdDaysBefore holds the days from from up to, and not including, the
+// day already held at until. holdDaysAfter holds the days after the day
+// already held at since, up to to.
+func holdDaysBefore(recorders []lease.RequestRecorder, from, until time.Time) {
+ from, until = from.UTC(), until.UTC()
+ lastNew := time.Date(until.Year(), until.Month(), until.Day(), 0, 0, 0, 0, time.UTC).Add(-time.Nanosecond)
+ if lastNew.Before(from) {
+ return
+ }
+ holdDays(recorders, from, lastNew)
+}
+
+func holdDaysAfter(recorders []lease.RequestRecorder, since, to time.Time) {
+ since, to = since.UTC(), to.UTC()
+ firstNew := time.Date(since.Year(), since.Month(), since.Day(), 0, 0, 0, 0, time.UTC).AddDate(0, 0, 1)
+ if firstNew.After(to) {
+ return
+ }
+ holdDays(recorders, firstNew, to)
+}
+
+// staleWindow is the span a fleet held, for the fleet that replaces it:
+// the union of what it found itself and what it inherited from the fleet
+// it replaced, so nothing is dropped between fleets, by a fleet that went
+// stale after inheriting, or by one closed without ever opening. A fleet
+// that has not yet inherited (its predecessor has not closed) answers for
+// its predecessor as well.
func (f *MeteredFleet) staleWindow() (from, at time.Time, stale bool) {
f.staleMu.Lock()
- defer f.staleMu.Unlock()
- if f.stale.Load() {
- return f.staleFrom, f.staleAt, true
+ own, ownFrom, ownAt := f.stale.Load(), f.staleFrom, f.staleAt
+ inherited, inhFrom, inhAt := f.inheritedStale, f.inheritedFrom, f.inheritedAt
+ latched := f.inherited.Load()
+ f.staleMu.Unlock()
+ if !latched && f.replaces != nil {
+ inhFrom, inhAt, inherited = f.replaces.staleWindow()
}
- if f.inheritedStale {
- return f.inheritedFrom, f.inheritedAt, true
+ switch {
+ case own && inherited:
+ return earlier(ownFrom, inhFrom), later(ownAt, inhAt), true
+ case own:
+ return ownFrom, ownAt, true
+ case inherited:
+ return inhFrom, inhAt, true
}
return time.Time{}, time.Time{}, false
}
+func earlier(a, b time.Time) time.Time {
+ if b.Before(a) {
+ return b
+ }
+ return a
+}
+
+func later(a, b time.Time) time.Time {
+ if b.After(a) {
+ return b
+ }
+ return a
+}
+
// inheritHold reads, once, the window the fleet this one replaces held,
// tells this fleet's recorders, and keeps it to pass on. It is called at
-// Open and at Close, whichever comes first, after the old fleet has closed
+// Open and at Close, and takes effect only once the old fleet has closed
// (closing asks the witness one last time, so whatever the old fleet had
-// to say about its last days it has said by then).
+// to say about its last days it has said by then): a fleet closed before
+// its predecessor does not latch, and answers for its predecessor through
+// staleWindow until a later fleet reads it.
func (f *MeteredFleet) inheritHold() {
- if f.replaces == nil || !f.inherited.CompareAndSwap(false, true) {
+ if f.replaces == nil || !f.replaces.closed.Load() || !f.inherited.CompareAndSwap(false, true) {
return
}
if from, at, stale := f.replaces.staleWindow(); stale {
@@ -343,8 +426,19 @@ func holdDays(recorders []lease.RequestRecorder, from, to time.Time) {
// replaces is still open: two replicators over one database share its
// shadow state and its replica prefix.
func (f *MeteredFleet) Open(ctx context.Context) error {
- if f.replaces != nil && !f.replaces.closed.Load() {
- return errors.New("ops: the replicator this one replaces has not been closed")
+ f.life.Lock()
+ defer f.life.Unlock()
+ // A closed fleet is as good as stale: stopped, to be rebuilt.
+ if f.closed.Load() {
+ return ErrOwnershipStale
+ }
+ // Every fleet this one replaces, however far back, must have closed:
+ // a fleet closed without opening passes its predecessor on, and two
+ // stores over one database would share its shadow state and prefix.
+ for prev := f.replaces; prev != nil; prev = prev.replaces {
+ if !prev.closed.Load() {
+ return errors.New("ops: the replicator this one replaces has not been closed")
+ }
}
// The fleet this one replaces has closed, and closing asks the witness
// one last time, so whatever it had to say about its last days it has
@@ -422,11 +516,15 @@ func (f *MeteredFleet) lookAgain() {
// Gone altogether: nothing is being shipped from it.
continue
}
- // Held from before this look began: the file may have changed at
- // any moment since the last look that found it as it was.
- f.goStaleFrom(began, time.Now())
+ // Held from before the last look that found every file as it was
+ // began: the file may have changed at any moment since, this
+ // look's own span included.
+ f.goStaleFrom(time.Unix(0, f.lastClean.Load()), time.Now())
return
}
+ // Every file is as it was: nothing before this look's start is in
+ // doubt when a later look finds a change.
+ f.lastClean.Store(began.UnixNano())
if inv.render() != f.inventory {
f.drift.Store(true)
}
@@ -472,6 +570,8 @@ func (f *MeteredFleet) Close() error {
}
func (f *MeteredFleet) closeNow() error {
+ f.life.Lock()
+ defer f.life.Unlock()
// The store first, and outside the fleet's lock: its workers ask the
// gate on their way out, and the gate takes the lock. A fleet whose
// witness was released while its store kept running would go on
@@ -481,6 +581,9 @@ func (f *MeteredFleet) closeNow() error {
store := f.store
f.store = nil
f.mu.Unlock()
+ if f.closingHook != nil {
+ f.closingHook()
+ }
var first error
if store != nil {
// Litestream's last act is to sync each replica, trying again for
@@ -579,6 +682,7 @@ func MeteredFleetStore(o MeteredFleetOptions) (fleet *MeteredFleet, err error) {
replaces: o.Replaces, stop: make(chan struct{}),
}
built.proven.Store(time.Now().UnixNano())
+ built.lastClean.Store(time.Now().UnixNano())
defer func() {
if err != nil {
built.Close()
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go
index ee61b385e..cb9ce15f8 100644
--- a/internal/ops/litestream_metered_test.go
+++ b/internal/ops/litestream_metered_test.go
@@ -1184,3 +1184,255 @@ func TestCloseRunsOnceForEveryCaller(t *testing.T) {
t.Fatal("the fleet is not closed after every caller returned")
}
}
+
+// A file swap found by the second look is held from the start of the last
+// look that found every file as it was, not from the look that found the
+// swap and not from the last registry proof: the swap may have been made
+// at any moment since.
+func TestAFileSwapIsHeldFromTheLastCleanLook(t *testing.T) {
+ newFakeBucket(t)
+ dir := platformDir(t)
+ reg := openRegistry(t, filepath.Join(dir, "registry.db"))
+ db := writeFile(t, filepath.Join(dir, "a.example.com.db"))
+ upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: db})
+ recs := &recorders{}
+ o := fleetOptions(reg, dir)
+ o.Meter = recs.meter
+ fleet, err := MeteredFleetStore(o)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer fleet.Close()
+ // The last clean look began two days ago; the registry was proved
+ // sound an hour ago; the swap is found now.
+ lastClean := time.Now().UTC().Add(-48 * time.Hour)
+ fleet.lastClean.Store(lastClean.UnixNano())
+ fleet.proven.Store(time.Now().Add(-time.Hour).UnixNano())
+ swapped := writeFile(t, filepath.Join(platformDir(t), "other.db"))
+ if err := os.Rename(swapped, db); err != nil {
+ t.Fatal(err)
+ }
+ fleet.lookAgain()
+ if !fleet.stale.Load() {
+ t.Fatal("the swap was not found")
+ }
+ want := lastClean.Format("2006-01-02")
+ for path, rec := range recs.all() {
+ if !rec.days()[want] {
+ t.Errorf("%s: the day the last clean look began (%s) is not held; held %v", path, want, rec.days())
+ }
+ }
+ // A second finding that reaches further back widens the hold, and a
+ // finding within what is held adds nothing.
+ a := &countingRecorder{}
+ f := &MeteredFleet{recorders: []lease.RequestRecorder{a}}
+ day := func(d int, h, m, s int) time.Time { return time.Date(2026, 10, d, h, m, s, 0, time.UTC) }
+ f.proven.Store(day(3, 0, 0, 30).UnixNano())
+ f.goStale(day(3, 0, 1, 0))
+ if got := a.days(); !got["2026-10-02"] || !got["2026-10-03"] || len(got) != 2 {
+ t.Fatalf("first finding held %v", got)
+ }
+ _, _, before := a.counts()
+ f.goStaleFrom(day(3, 0, 0, 0), day(3, 0, 2, 0))
+ if _, _, after := a.counts(); after != before {
+ t.Fatalf("a finding within the held window held more (%d -> %d)", before, after)
+ }
+ f.goStaleFrom(day(1, 12, 0, 0), day(3, 0, 2, 0))
+ if got := a.days(); !got["2026-10-01"] {
+ t.Fatalf("a finding further back did not widen the hold: %v", got)
+ }
+}
+
+// What a fleet passes on is the union of what it found itself and what it
+// inherited: a replacement that went stale after inheriting passes both
+// on; and a fleet closed before its predecessor does not latch an empty
+// inheritance, so the predecessor's later finding still reaches the fleet
+// after it.
+func TestInheritedWindowIsTheUnion(t *testing.T) {
+ day := func(d int) time.Time { return time.Date(2026, 10, d, 12, 0, 0, 0, time.UTC) }
+ hold := func(d int) *MeteredFleet {
+ f := &MeteredFleet{}
+ f.proven.Store(day(d).UnixNano())
+ f.goStale(day(d))
+ f.closed.Store(true)
+ return f
+ }
+ a := hold(2)
+ b := &MeteredFleet{replaces: a}
+ b.inheritHold()
+ b.proven.Store(day(4).UnixNano())
+ b.goStale(day(4))
+ b.closed.Store(true)
+ from, at, stale := b.staleWindow()
+ if !stale || from.After(day(2)) || at.Before(day(4)) {
+ t.Fatalf("B's window %v..%v %v; want the union of its own and A's", from, at, stale)
+ }
+ c := &countingRecorder{}
+ cf := &MeteredFleet{replaces: b, recorders: []lease.RequestRecorder{c}}
+ cf.inheritHold()
+ if got := c.days(); !got["2026-10-02"] || !got["2026-10-04"] {
+ t.Fatalf("C holds %v; want A's day and B's", got)
+ }
+ // B closed while A was still open: B does not latch, and C, built on
+ // B, reads A's finding through B when it comes.
+ a2 := &MeteredFleet{}
+ b2 := &MeteredFleet{replaces: a2}
+ b2.inheritHold() // A is open: nothing latched
+ b2.closed.Store(true)
+ a2.proven.Store(day(2).UnixNano())
+ a2.goStale(day(2))
+ a2.closed.Store(true)
+ c2 := &countingRecorder{}
+ cf2 := &MeteredFleet{replaces: b2, recorders: []lease.RequestRecorder{c2}}
+ cf2.inheritHold()
+ if got := c2.days(); !got["2026-10-02"] {
+ t.Fatalf("C holds %v; want A's day, through a B that closed first", got)
+ }
+}
+
+// Open refuses while any fleet in the chain this one replaces is still
+// open, not only the nearest.
+func TestOpenRefusesWhileAnyPredecessorIsOpen(t *testing.T) {
+ newFakeBucket(t)
+ dir := platformDir(t)
+ reg := openRegistry(t, filepath.Join(dir, "registry.db"))
+ db := writeFile(t, filepath.Join(dir, "a.example.com.db"))
+ upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: db})
+ o := fleetOptions(reg, dir)
+ a, err := MeteredFleetStore(o)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer a.Close()
+ o.Replaces = a
+ b, err := MeteredFleetStore(o)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := b.Close(); err != nil {
+ t.Fatal(err)
+ }
+ o.Replaces = b
+ c, err := MeteredFleetStore(o)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer c.Close()
+ if err := c.Open(context.Background()); err == nil || !strings.Contains(err.Error(), "not been closed") {
+ t.Fatalf("C opened while A, two fleets back, was open: %v", err)
+ }
+ if err := a.Close(); err != nil {
+ t.Fatal(err)
+ }
+ if err := c.Open(context.Background()); err != nil {
+ t.Fatalf("C after A closed: %v", err)
+ }
+}
+
+// A second Close caller waits for the one shutdown, and a replacement
+// cannot open, until the first shutdown has finished.
+func TestCloseBlocksEveryCallerUntilTheStoreHasStopped(t *testing.T) {
+ newFakeBucket(t)
+ dir := platformDir(t)
+ reg := openRegistry(t, filepath.Join(dir, "registry.db"))
+ db := writeFile(t, filepath.Join(dir, "a.example.com.db"))
+ upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: db})
+ o := fleetOptions(reg, dir)
+ fleet, err := MeteredFleetStore(o)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := fleet.Open(context.Background()); err != nil {
+ t.Fatal(err)
+ }
+ entered, release := make(chan struct{}), make(chan struct{})
+ fleet.closingHook = func() {
+ close(entered)
+ <-release
+ }
+ first := make(chan error, 1)
+ go func() { first <- fleet.Close() }()
+ <-entered
+ second := make(chan error, 1)
+ go func() { second <- fleet.Close() }()
+ o.Replaces = fleet
+ next, err := MeteredFleetStore(o)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer next.Close()
+ select {
+ case <-second:
+ t.Fatal("a second Close returned while the first shutdown was still running")
+ case <-time.After(300 * time.Millisecond):
+ }
+ if err := next.Open(context.Background()); err == nil || !strings.Contains(err.Error(), "not been closed") {
+ t.Fatalf("a replacement opened while the shutdown was running: %v", err)
+ }
+ if fleet.closed.Load() {
+ t.Fatal("the fleet reads as closed while its store is still being closed")
+ }
+ close(release)
+ if err := <-first; err != nil {
+ t.Fatal(err)
+ }
+ if err := <-second; err != nil {
+ t.Fatal(err)
+ }
+ if !fleet.closed.Load() {
+ t.Fatal("not closed after both callers returned")
+ }
+ if err := next.Open(context.Background()); err != nil {
+ t.Fatalf("the replacement after the shutdown: %v", err)
+ }
+}
+
+// A reading of the witness that fails is tried again before it counts as
+// a change: a recovered error admits; errors that keep coming make the
+// fleet stale; a witnessed change makes it stale at once.
+func TestWitnessReadsAreRetriedBeforeTheyCount(t *testing.T) {
+ newFakeBucket(t)
+ for name, tc := range map[string]struct {
+ answers []error
+ changed bool
+ want error
+ }{
+ "a recovered error": {[]error{errors.New("busy"), errors.New("busy"), nil}, false, nil},
+ "errors that keep on": {[]error{errors.New("busy"), errors.New("busy"), errors.New("busy")}, false, ErrOwnershipStale},
+ "a change": {[]error{nil}, true, ErrOwnershipStale},
+ "a change after error": {[]error{errors.New("busy"), nil}, true, ErrOwnershipStale},
+ } {
+ t.Run(name, func(t *testing.T) {
+ dir := platformDir(t)
+ reg := openRegistry(t, filepath.Join(dir, "registry.db"))
+ db := writeFile(t, filepath.Join(dir, "a.example.com.db"))
+ upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: db})
+ fleet, err := MeteredFleetStore(fleetOptions(reg, dir))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer fleet.Close()
+ calls := 0
+ fleet.witness = func(context.Context) (bool, error) {
+ i := calls
+ if i >= len(tc.answers) {
+ i = len(tc.answers) - 1
+ }
+ calls++
+ if tc.answers[i] != nil {
+ return false, tc.answers[i]
+ }
+ return !tc.changed, nil
+ }
+ if err := fleet.admit(context.Background()); !errors.Is(err, tc.want) {
+ t.Fatalf("admit: %v, want %v", err, tc.want)
+ }
+ if tc.want == nil && calls != len(tc.answers) {
+ t.Fatalf("%d reads, want %d", calls, len(tc.answers))
+ }
+ if tc.want != nil && !tc.changed && calls != witnessAttempts {
+ t.Fatalf("%d reads before giving up, want %d", calls, witnessAttempts)
+ }
+ })
+ }
+}
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go
index 86aacd658..78de12e50 100644
--- a/internal/ops/litestream_owners.go
+++ b/internal/ops/litestream_owners.go
@@ -326,37 +326,29 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) {
if !ok {
return fileFacts{}, "file ownership cannot be read on this platform"
}
- // The file's own directory first: it is the bottom of the chain
- // and has to be the owner's (or the platform's) like every other
- // directory in it. A state directory the platform made as a link
- // (systemd's DynamicUser shape: /var/lib/carlos-app/<host> is a
- // root-made link to /var/lib/private/...) is followed when the
- // link itself, its parent and the resolved directory's parent are
- // all the platform's; the chain is then the resolved one. A link
- // the tenant made (its own, in its own directory) is refused.
+ // The spelled directory path is resolved first, one component at
+ // a time: a link the platform made (systemd's DynamicUser shape:
+ // /var/lib/carlos-app/<host> is a root-made link to
+ // /var/lib/private/...) is followed when the link itself and its
+ // parent are the platform's, its target is absolute and holds no
+ // further link, and the target's parent is the platform's; the
+ // chain is then the resolved one, and a file named beneath such a
+ // link at any depth is owned by its resolved path. A link anywhere
+ // else in the path, the tenant's or anyone's, is refused.
+ resolved, why := resolvePlatformLinks(root)
+ if why != "" {
+ return fileFacts{}, why
+ }
+ if resolved != root {
+ root, path = resolved, filepath.Join(resolved, filepath.Base(path))
+ }
+ // The file's own directory next: it is the bottom of the chain and
+ // has to be the owner's (or the platform's, and then writable by
+ // nobody else) like every other directory in it.
start, serr := os.Lstat(root)
if serr != nil {
return fileFacts{}, "its directory cannot be looked at"
}
- if start.Mode()&os.ModeSymlink != 0 {
- if why := platformOwned(filepath.Dir(root)); why != "" {
- return fileFacts{}, "its directory is a link in a directory that is not the platform's: " + why
- }
- if lf, ok := factsOf(start); !ok || (lf.UID != 0 && int(lf.UID) != os.Geteuid()) {
- return fileFacts{}, "its directory is a link that is not the platform's"
- }
- real, rerr := filepath.EvalSymlinks(root)
- if rerr != nil {
- return fileFacts{}, "its directory is a link that cannot be resolved"
- }
- if why := platformOwned(filepath.Dir(real)); why != "" {
- return fileFacts{}, "its directory is a link to a directory that is not under the platform's: " + why
- }
- root, path = real, filepath.Join(real, filepath.Base(path))
- if start, serr = os.Lstat(root); serr != nil {
- return fileFacts{}, "its directory cannot be looked at"
- }
- }
sf, ok := factsOf(start)
if !ok {
return fileFacts{}, "file ownership cannot be read on this platform"
@@ -364,7 +356,14 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) {
if !start.IsDir() || start.Mode()&os.ModeSymlink != 0 {
return fileFacts{}, "its directory is not a directory"
}
- if sf.UID != ownerFacts.UID && sf.UID != 0 && int(sf.UID) != os.Geteuid() {
+ if sf.UID == 0 || int(sf.UID) == os.Geteuid() {
+ // The platform's own directory (the file may be the platform's
+ // too): writable by nobody else, or a local user could put a
+ // sidecar link beside the file.
+ if start.Mode().Perm()&0o022 != 0 {
+ return fileFacts{}, "its directory is the platform's and can be written by its group or by anyone"
+ }
+ } else if sf.UID != ownerFacts.UID {
return fileFacts{}, "its directory belongs to a user other than its own or the platform's"
}
for {
@@ -385,6 +384,14 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) {
if top, terr := os.Lstat(root); terr != nil || top.Mode()&os.ModeSymlink != 0 || !top.IsDir() {
return fileFacts{}, "the top of its directory chain is a link or cannot be looked at"
}
+ // Nothing above the top of the chain is a link either: a link
+ // higher in the spelled path would have been resolved above under
+ // the platform's rules or refused, so the resolved and spelled
+ // paths now agree, and a tenant's link above the chain (one that
+ // leads into another tenant's tree) is caught here.
+ if real, rerr := filepath.EvalSymlinks(root); rerr != nil || filepath.Clean(real) != filepath.Clean(root) {
+ return fileFacts{}, "a directory above its chain is a link"
+ }
if why := platformOwned(filepath.Dir(root)); why != "" {
return fileFacts{}, why
}
@@ -567,20 +574,82 @@ func sqliteName(name string) bool {
// sidecarOf names the database a file would be the sidecar of (its -wal,
// -shm or -journal), when that database exists beside it as a regular
-// file; "" otherwise.
+// file that is a database (by name, or by its header); "" otherwise. A
+// regular file beside it that is not a database makes it no sidecar, and
+// it is read for its own header like any other file.
func sidecarOf(p string) string {
- lower := strings.ToLower(p)
for _, suffix := range []string{"-wal", "-shm", "-journal"} {
- if strings.HasSuffix(lower, suffix) {
- base := p[:len(p)-len(suffix)]
- if fi, err := os.Lstat(base); err == nil && fi.Mode().IsRegular() {
- return base
- }
+ if !strings.HasSuffix(p, suffix) {
+ continue
+ }
+ base := p[:len(p)-len(suffix)]
+ fi, err := os.Lstat(base)
+ if err != nil || !fi.Mode().IsRegular() {
+ return ""
+ }
+ if sqliteName(filepath.Base(base)) {
+ return base
}
+ if isDB, herr := sqliteHeader(base, fi); herr == nil && isDB {
+ return base
+ }
+ return ""
}
return ""
}
+// resolvePlatformLinks resolves the links the platform made in a spelled
+// directory path, one component at a time, and refuses any other link. A
+// link is the platform's when the link itself and the directory holding
+// it are the platform's, its target is an absolute path holding no further
+// link, and the target's parent is the platform's. The result is a path
+// with no link in it, or the reason it could not be.
+func resolvePlatformLinks(dir string) (string, string) {
+ dir = filepath.Clean(dir)
+ if !filepath.IsAbs(dir) {
+ return "", "its directory is not an absolute path"
+ }
+ parts := strings.Split(strings.TrimPrefix(dir, string(filepath.Separator)), string(filepath.Separator))
+ prefix := string(filepath.Separator)
+ for i := 0; i < len(parts); i++ {
+ if parts[i] == "" {
+ continue
+ }
+ next := filepath.Join(prefix, parts[i])
+ fi, err := os.Lstat(next)
+ if err != nil {
+ return "", "a directory in its path cannot be looked at"
+ }
+ if fi.Mode()&os.ModeSymlink == 0 {
+ prefix = next
+ continue
+ }
+ if why := platformOwned(prefix); why != "" {
+ return "", "a link in its path is in a directory that is not the platform's: " + why
+ }
+ if lf, ok := factsOf(fi); !ok || (lf.UID != 0 && int(lf.UID) != os.Geteuid()) {
+ return "", "a link in its path is not the platform's"
+ }
+ target, rerr := os.Readlink(next)
+ if rerr != nil || !filepath.IsAbs(target) {
+ return "", "a link in its path is not an absolute link"
+ }
+ target = filepath.Clean(target)
+ real, eerr := filepath.EvalSymlinks(target)
+ if eerr != nil || filepath.Clean(real) != target {
+ return "", "a link in its path leads through another link"
+ }
+ if tfi, terr := os.Lstat(target); terr != nil || !tfi.IsDir() {
+ return "", "a link in its path does not lead to a directory"
+ }
+ if why := platformOwned(filepath.Dir(target)); why != "" {
+ return "", "a link in its path leads to a directory that is not under the platform's: " + why
+ }
+ prefix = target
+ }
+ return prefix, ""
+}
+
// sqliteHeader reports whether the file begins with SQLite's header. A
// file too short to hold one is not a database; a file that cannot be
// opened or read is an error, which the caller treats as a place it could
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go
index 5de9be7a9..5cb6611e9 100644
--- a/internal/ops/litestream_owners_test.go
+++ b/internal/ops/litestream_owners_test.go
@@ -521,4 +521,14 @@ func TestStrictTreeReadsAStandaloneSidecarName(t *testing.T) {
if named(skipped, beside+"-wal", "") || named(problems, beside+"-wal", "") {
t.Fatalf("a real sidecar is reported: skipped %v problems %v", skipped, problems)
}
+ // A regular file beside it that is not a database makes it no sidecar:
+ // "archive" (text) and "archive-journal" (a database) are a text file
+ // and a database, and the database is reported.
+ if err := os.WriteFile(filepath.Join(root, "archive"), []byte("notes, not a database"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ _, skipped, problems = strictTreeDBs(root)
+ if !named(skipped, alone, "") {
+ t.Fatalf("a database named archive-journal beside a text file named archive is not reported: skipped %v problems %v", skipped, problems)
+ }
}
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go
index 6efe6d4f7..7495cd2f7 100644
--- a/internal/ops/litestream_owners_unix_test.go
+++ b/internal/ops/litestream_owners_unix_test.go
@@ -124,9 +124,88 @@ func TestUnitFileThroughAPlatformMadeStateLink(t *testing.T) {
}
// The same link made by the tenant is refused.
withOwners(t, map[string]uint32{"carlos-app": 0, "private": 0, "keymail.dev": tenant, "registry.db": tenant, "link:keymail.dev": tenant})
- if _, why := checkOwnedFile(filepath.Dir(spelled), spelled, tenantFile); !strings.Contains(why, "link that is not the platform's") {
+ if _, why := checkOwnedFile(filepath.Dir(spelled), spelled, tenantFile); !strings.Contains(why, "not the platform's") {
t.Fatalf("the tenant's own link in the platform's directory: %q", why)
}
+ // A file named beneath the platform's link at any depth is owned by its
+ // resolved path too (the link is resolved wherever it sits in the
+ // spelled path, not only as the file's own directory).
+ withOwners(t, map[string]uint32{"carlos-app": 0, "private": 0, "keymail.dev": tenant, "data": tenant, "nested.db": tenant, "link:keymail.dev": 0})
+ nested := writeFile(t, filepath.Join(private, "keymail.dev", "data", "nested.db"))
+ spelledNested := filepath.Join(link, "data", "nested.db")
+ if facts, why := checkOwnedFile(filepath.Dir(spelledNested), spelledNested, tenantFile); why != "" {
+ t.Fatalf("a file nested beneath the platform's link: %q", why)
+ } else if nfi, _ := os.Lstat(nested); nfi != nil {
+ if nf, _ := statFacts(nfi); nf.Ino != facts.Ino {
+ t.Fatal("the facts are not the nested file's")
+ }
+ }
+ // A link the platform made whose target leads through a link the
+ // tenant controls is refused: the target must hold no further link.
+ withOwners(t, map[string]uint32{"carlos-app": 0, "private": 0, "keymail.dev": tenant, "registry.db": tenant, "link:keymail.dev": 0, "link:choice": tenant, "hop": tenant})
+ hop := filepath.Join(top, "hop")
+ if err := os.MkdirAll(hop, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(filepath.Dir(real), filepath.Join(hop, "choice")); err != nil {
+ t.Fatal(err)
+ }
+ via := filepath.Join(public, "via")
+ if err := os.Symlink(filepath.Join(hop, "choice"), via); err != nil {
+ t.Fatal(err)
+ }
+ withOwners(t, map[string]uint32{"carlos-app": 0, "private": 0, "keymail.dev": tenant, "registry.db": tenant, "link:via": 0, "link:choice": tenant, "hop": tenant})
+ if _, why := checkOwnedFile(filepath.Dir(filepath.Join(via, "registry.db")), filepath.Join(via, "registry.db"), tenantFile); !strings.Contains(why, "through another link") {
+ t.Fatalf("a platform link whose target passes through the tenant's link: %q", why)
+ }
+}
+
+// A link above the top of a tenant's chain, inside the spelled path, is
+// refused even when the chain's own directories are all the tenant's: the
+// spelled and the resolved path must agree, or another tenant's tree is
+// reached under this tenant's route.
+func TestUnitFileChainRefusesALinkAboveIt(t *testing.T) {
+ const tenant, other = 44001, 44002
+ top := platformDir(t)
+ apps := filepath.Join(top, "apps")
+ private := filepath.Join(top, "private")
+ for _, d := range []string{apps, private} {
+ if err := os.MkdirAll(d, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ }
+ // V's tree, under a platform directory; T's directory holds a link to
+ // the platform directory above V's tree.
+ theirs := writeFile(t, filepath.Join(private, "V", "main.db"))
+ if err := os.MkdirAll(filepath.Join(apps, "T"), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink(private, filepath.Join(apps, "T", "a")); err != nil {
+ t.Fatal(err)
+ }
+ withOwners(t, map[string]uint32{"apps": 0, "private": 0, "T": tenant, "link:a": tenant, "V": other, "main.db": other})
+ spelled := filepath.Join(apps, "T", "a", "V", "main.db")
+ if _, why := checkOwnedFile(filepath.Dir(spelled), spelled, tenantFile); why == "" {
+ t.Fatalf("V's file reached through T's link was accepted (%s)", theirs)
+ }
+}
+
+// A unit file in a platform-owned directory that anyone can write to is
+// refused: a local user could put a sidecar link beside it.
+func TestUnitFileInAWorldWritablePlatformDirectory(t *testing.T) {
+ top := platformDir(t)
+ shared := filepath.Join(top, "shared")
+ if err := os.MkdirAll(shared, 0o1777); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Chmod(shared, 0o1777); err != nil {
+ t.Fatal(err)
+ }
+ withOwners(t, map[string]uint32{"shared": 0, "x.db": 0})
+ db := writeFile(t, filepath.Join(shared, "x.db"))
+ if _, why := checkOwnedFile(filepath.Dir(db), db, tenantFile); !strings.Contains(why, "can be written by its group or by anyone") {
+ t.Fatalf("a platform-owned file in a world-writable directory: %q", why)
+ }
}
// A FIFO put where a file was looked at does not hold the walk: the header