amadan
The replicator's eleventh build: the tenth review's findings
A file is checked by the strictest claim on it: an exec route's database outside the activator's directories is a tenant's file (both reviewers: the tenth build checked it as a named file and followed a tenant's directory link to another tenant's file); a hibernating path that the platform or any live route also claims is a problem; a route over the platform's own database is unowned; Close's look holds from the last clean look when it cannot be taken; a link in a unit tree to a regular file is named to the caller. Tests pin the latch before the closed flag and O_NOFOLLOW against the identity check. Dispositions "Review ten" and the tenth review's reports.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
9 files changed,
+622
−57
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review10.md+170 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+22 −4docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review10.md+113 −0internal/ops/litestream.go+27 −8internal/ops/litestream_metered.go+23 −12internal/ops/litestream_metered_test.go+70 −8internal/ops/litestream_owners.go+86 −22internal/ops/litestream_owners_test.go+40 −3internal/ops/litestream_owners_unix_test.go+71 −0
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review10.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review10.md| new file mode 100644 |
| index 000000000..861999960 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review10.md |
| @@ -0,0 +1,170 @@ |
| +| Review-nine finding | Closed at `9301ed62`? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 — no final file look at Close** | **Yes, for the reported persistent swap.** `closeNow` calls `lookNow` after stopping the store and watch. | **No.** `TestCloseLooksAtTheFilesOneLastTime` checks staleness, the exact lower bound and recorder holds. It does not test shutdown ordering on an opened store. | |
| +| **Opus P3-1 — root/platform-UID tenants** | **No.** Still an unenforced configuration precondition. | No rejecting regression test exists. Several fixtures deliberately accept the process UID. | |
| +| **Opus P3-2 — hibernation suppresses another claim** | **Partly.** Exact unit-file and platform-path conflicts are reported. Unit-tree children and conflicting non-hibernating exec claims still disappear. | **No** for removing the added report: the new exact-unit-file case fails. **Yes** for the remaining omissions: that fixture misses them. | |
| +| **Opus P3-3 — routine restart loses inheritance** | **Documentation fixed; behavior open.** The comment now includes any process exit. | No restart-recovery test establishes persistence. | |
| +| **Opus P3-4 — weak tests** | **Partly.** Inheritance inside `Close` and witnessed-change retry counts are now asserted. Other gaps remain below. | Removing `inheritHold` from `closeNow` fails the new unopened-fleet test. Unnecessary retries after a witnessed change now fail. Removing several other protections remains undetected. | |
| +| **Opus claims** | **Mostly corrected.** Registry wording, non-Unix refusal, climb qualification and latch explanation are corrected. The strict-walk promise remains too broad; Close comments remain inaccurate. | Prose corrections have no behavioral test. Author-reported test results were not verified here. | |
| +| **Astra 1 — Litestream pathname opens** | **No.** Initialization, subsequent opens and WAL opens remain exposed. | Existing tests do not force substitution at those boundaries. | |
| +| **Astra 2 — replaceable ancestry** | **No.** Protected endpoints do not establish protected access paths. | Existing endpoint fixtures still pass with higher ancestry unchecked. | |
| +| **Astra 3 — delayed admission escapes held days** | **No.** The one-minute upper margin remains unbounded by request lifetime. | No test exercises the delayed-admission sequence below. | |
| +| **Astra 4 — crash loses ownership doubt** | **No.** Transition state remains in memory; routine restart also loses it. | No test establishes recovery of a completed, doubtful request. | |
| +| **Astra 5 — explicit exec claims omitted** | **Closed for an uncontested absolute claim.** The broader inventory finding is only partly closed, and accepting arbitrary claims exposes finding 1 below. | **No.** Removing explicit enumeration fails `TestExplicitClaimsOutsideTheGlobsAreInventoried`. Its fixtures do not challenge directory authority or conflicting hibernation. | |
| +| **Astra 6 — header probe opens a device** | **No.** Ancestor substitution can redirect the privileged open. | The final-component tests do not exercise this. | |
| +| **Astra 7 — filesystem UID not bound to route** | **No.** The expected UID is still inferred from the claimed file/tree. | No test requires rejecting an otherwise valid tree belonging to a different route’s user. | |
| +| **Astra 8 — tests and environmental assumptions** | **Partly.** Retry counts improve; the two directory fixtures now use `mkdirs`. Shutdown, attribution, watch, timing and synthetic-UID limitations remain. | Several individual mutations still pass, detailed below. | |
| +| **Astra claims** | **Mostly corrected.** Repeated holding, disappearance classification, test pathname wording, sidecar wording and bind-mount qualification are repaired. Remaining contradictions are listed last. | Source inspection establishes these corrections, not execution results. | |
| + |
| +All locations below refer to **`9301ed62`**. I read both review-eight and review-nine reports from that commit. Test counterfactuals are source-based; no files were changed, tests run, or network commands issued. |
| + |
| +1. **P1 — Newly enumerated exec claims can accept another tenant’s file through an unchecked directory link.** |
| + |
| + **Locations:** `internal/ops/litestream.go:389`, `:454`; `internal/ops/litestream_owners.go:178`, `:282`, `:438`. |
| + |
| + Configure a non-hibernating exec route for A with `DBPath=/srv/A/live/main.sqlite`, outside the discovery globs. A replaces `live` with a symlink to B’s database directory. B’s database is not independently listed—for example, B is hibernating. |
| + |
| + The new explicit enumeration includes A’s pathname. Attribution selects `namedFile`; `os.Stat(root)` follows the directory link. The subsequent walk checks only `main.sqlite`, and the UID comparison applies only to `tenantTree` and `tenantFile`. B’s regular, single-link file and ordinary siblings therefore pass as A’s database. No race is needed, and duplicate-file detection cannot help when only A’s alias is listed. |
| + |
| + This is the directory assumption previously confined to activator-managed paths, now applied to arbitrary operator-selected paths. A `tenantFile` claim through the same tenant-made directory link would be refused. |
| + |
| + **Smallest acceptable fix:** permit `namedFile` only beneath an independently verified activator-controlled root. Give outside-root claims equivalent directory and owner validation, or refuse them pending an explicit ownership model. Add the stable parent-link case to the explicit-claim test. |
| + |
| +2. **P1 — The checks still do not constrain the files Litestream actually opens.** |
| + |
| + **Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:268`; `third_party/litestream/db.go:1004`, `:1014`, `:1032`, `:1866`. |
| + |
| + A passes inventory, substitutes B’s database while Litestream opens its SQLite connection and persistent descriptor, then restores A’s pathname before the next look. Later inventories find A’s original inode; Litestream can retain B’s descriptor. Initialization can also change the substituted database’s journal mode and create bookkeeping tables. |
| + |
| + The new Close look does not detect a substitution restored before that look. Subsequent WAL opens remain exposed independently of initialization. |
| + |
| + **Smallest acceptable fix:** carry validated, pinned directory authority through the actual SQLite, database and sidecar opens, including reopen paths. Check opened identities as additional detection; checking them after an unrestricted open does not prevent its effects. |
| + |
| +3. **P1 — Platform-owned endpoints can still sit beneath tenant-replaceable ancestors.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:397`, `:462`, `:647`, `:675`; `internal/ops/litestream_metered.go:933`. |
| + |
| + Let T own `/srv/T`, containing root-owned `bridge`, which contains a tenant state directory. A platform-made link targets `/srv/T/bridge/A`. The immediate parent checks pass, but T can rename `bridge` through `/srv/T` and substitute the hierarchy. |
| + |
| + Likewise, a protected `dataDir` beneath `/srv/T/bridge` passes `platformOwned`; the private `.replicator` directories below it do not protect their replaceable ancestry. |
| + |
| + A platform link whose **immediate target parent** is tenant-owned is refused. The surviving case is higher ancestry, including ancestry above the link’s own protected parent. |
| + |
| + **Smallest acceptable fix:** establish and pin a trusted ancestry boundary for claims, authorized links and shadow state. Reject tenant-replaceable access paths before creating or opening state. |
| + |
| +4. **P1 — An admitted request can land on a day absent from every inherited window.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:237`, `:322`, `:747`; `internal/lease/requestgate.go:58`. |
| + |
| + A concrete sequence, in UTC: |
| + |
| + - A admits a request on October 2 at **23:58:00**, then it pauses before recording. |
| + - Ownership changes at **23:58:10**. A detects it and holds through **23:59:10**. |
| + - B is built for the new owner and closed unopened while A remains live. B correctly leaves its inheritance unlatched. |
| + - A’s admitted request resumes at **00:02 on October 3**, is recorded and sent. |
| + - A closes. Already stale, it skips both final checks and does not extend its window. |
| + - C replaces B. B and C correctly receive A’s window—but that window contains no October 3. |
| + |
| + The lower bound correctly uses the earlier registry proof and clean file look. The missing day comes from the upper bound, not the repaired inheritance chain. The tenth build’s final look does not change this sequence. |
| + |
| + **Smallest acceptable fix:** track admitted attempts through recording/completion and include their doubtful dates before allowing the handover window to become final. Increasing a fixed margin is insufficient. |
| + |
| +5. **P1 — Ownership doubt still disappears across process exit.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:343`, `:376`, `:792`. |
| + |
| + A request passes admission, ownership changes, and the request completes before invalidation. The process exits. Its successor observes the new owner with no predecessor and no persisted uncertainty requiring either owner’s completed day to be held. |
| + |
| + A clean restart also loses an in-memory window that should reach a new owner’s recorder. An unfinished ledger row addresses a different case. |
| + |
| + **Smallest acceptable fix:** durably record admission/ownership uncertainty before affected requests can escape, then recover it before replacement coverage is established. The comment’s reference to box-wide ledger incompleteness is not fleet-local recovery. |
| + |
| +6. **P2 — The hibernation conflict fix misses directory claims and active exec claims.** |
| + |
| + **Locations:** `internal/ops/litestream.go:475`, `:538`, `:544`. |
| + |
| + A unit route claims `/var/lib/carlos-app/U`. The strict walk discovers `/var/lib/carlos-app/U/main.db`. A hibernating route names that file exactly. |
| + |
| + `hibernating[p]` is true, but `unitPaths[p]` is empty: that map contains the **directory**, not the discovered file. `platformPaths[p]` is also false. The code silently continues before ownership conflict detection. The file reaches none of `Owned`, `Unowned`, `Skipped` or `Problems`. |
| + |
| + The same omission occurs when hibernating and non-hibernating exec routes name the same outside-glob file: `explicit[p]` is not consulted by the conflict check. |
| + |
| + **Smallest acceptable fix:** retain all candidate claim origins and reject hibernation suppression whenever a platform, unit-tree, unit-file or non-hibernating claim also covers the file. Test both sequences; the new test covers only an exact unit-file claim. |
| + |
| +7. **P2 — Header inspection can cause device-open effects before rejecting the descriptor.** |
| + |
| + **Location:** `internal/ops/sqliteopen_unix.go:26`. |
| + |
| + The walker observes regular file `T/d/watchdog`. T replaces `d` with a link to `/dev`. On a box exposing a watchdog device, the privileged open can reach `/dev/watchdog`. The subsequent regular-file and inode check cannot undo the open’s effects. |
| + |
| + `O_NOFOLLOW` protects the final component. `O_NONBLOCK` addresses FIFO blocking; `O_NOCTTY` prevents controlling-terminal acquisition. Neither establishes safe ancestor resolution. The non-Unix implementation correctly refuses header reads. |
| + |
| + **Smallest acceptable fix:** constrain resolution through pinned directory descriptors before the effects-bearing open, and establish acceptable file type safely. Preserve refusal on unsupported platforms. |
| + |
| +8. **P2 — Strict inventory still has silent exclusions, with no external coverage comparison.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:513`, `:525`, `:533`, `:544`; `internal/ops/litestream.go:523`. |
| + |
| + A unit tree contains `current -> /elsewhere/live.sqlite`. Because `current` has no recognized database suffix and its target is a regular file, it is silently ignored. If the existing external replicator streams that path, the new inventory supplies no entry requiring acknowledgement before coverage shrinks. |
| + |
| + Empty tree-only files and entries disappearing before their first `Lstat` also remain unlisted. An absent configured console database is indistinguishable from an optional console database that never existed. |
| + |
| + The bounded reports work: relative unit claims, units without `DBPath`, missing unit claims, empty explicit candidates, and discovered candidates failing their final look are reported. Case variants are recognized; uppercase `.DB` is skipped rather than replicated. Header reads cover nonempty regular files not already classified by name, including sidecar-shaped names. `sidecarOf` is gone. |
| + |
| + **Smallest acceptable fix:** give potentially database-bearing links an explicit disposition, distinguish required platform files from optional ones, and require every external-inventory entry to match an owned or explicitly accepted entry before cutover. |
| + |
| +9. **P3 — Unit ownership remains inferred from the file rather than bound to the route.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:325`, `:360`, `:380`, `:438`. |
| + |
| + A root-run unit route naming `/var/lib/fail2ban/fail2ban.db` passes when the file and protected parent directories meet the checks. The climb stops at the immediate root-owned parent; it does not climb to `/` and refuse. |
| + |
| + A tenant sharing the platform UID similarly qualifies for platform directory and link treatment. This is not acceptance literally “from anywhere”: file type, link count, directory permissions, parent ownership, device and sibling checks still apply. |
| + |
| + With a genuinely distinct file-owner UID, a stable `tenantFile` chain rejects a foreign non-platform starting directory, a foreign stopping parent and tenant-made links. A protected platform-owned starting directory is an explicit exception. Every accepting `tenantFile` path reaches the top-of-chain checks at `:385` and `:394`; ordinary static tenant links are usually refused earlier by the resolver. |
| + |
| + Nevertheless, route A can directly name B’s otherwise valid tree because no independent A-to-UID binding exists. A final-component `DBPath` symlink is refused; directly naming B’s regular file is a different case. |
| + |
| + **Smallest acceptable fix:** bind non-platform unit routes to independently established state roots and UIDs; reject root/platform-UID tenants except explicitly trusted platform workloads. Deployment approval remains separate. |
| + |
| +10. **P3 — Several tests still establish weaker properties than their comments claim.** |
| + |
| + **Locations:** `internal/ops/litestream_metered_test.go:687`, `:979`, `:1095`, `:1371`, `:1751`; `internal/ops/litestream_owners_unix_test.go:20`, `:252`. |
| + |
| + Specific counterfactuals: |
| + |
| + - Removing `store.Close` still satisfies shutdown assertions based on the cleared pointer and refusing gate. Retained handles and worker termination are not asserted. |
| + - Removing the watch’s file-look invocation leaves tests that call `lookAgain` directly intact. |
| + - Swapping database recorders can satisfy the opened-store test’s positive counts. Waiting for server inactivity does not compare received attempts with correctly attributed attempts. |
| + - The older lower-bound test still holds yesterday if `lookStart` is ignored: its registry proof minus the margin already reaches yesterday. The newer last-clean test distinguishes that defect. |
| + - Removing `O_NOFOLLOW` still rejects the link fixture because its target has a different inode. Use a link to the originally observed inode to distinguish no-follow from identity checking. |
| + - The new unopened-Close test pins inheritance **inside Close**, but moving inheritance after `closed.Store(true)` can still satisfy its after-return assertions. |
| + - Removing the latch’s under-lock recheck is not challenged by sequential successor tests. That concurrency is outside the documented single-chain caller contract. |
| + - The shadow `dataDir` fixture is rejected by inventory before reaching `prepareShadowState`’s own check. |
| + |
| + Under umask `077`, relevant modes are explicitly restored. `platformDir` resolves a symlinked temp-root spelling. Remaining assumptions include fixed synthetic UIDs `44001/44002`, basename-based ownership substitution, the 300 ms negative scheduling check, integration timing windows and wall-clock comparisons. Running as a synthetic tenant UID can change which ownership branch the fixture exercises. |
| + |
| + **Smallest acceptable fix:** use retained shutdown references, per-database attempt correspondence, controlled interleavings and fixture-scoped identities. Keep the stronger tests already added; do not treat redundant defensive checks as independently proven. |
| + |
| +The remaining requested checks hold with these qualifications: |
| + |
| +- **Admission and retries:** the pre-lock, under-lock and post-witness stale checks close the reported interleavings (`litestream_metered.go:211`, `:223`, `:237`). A finding during the witness read refuses admission. A finding after the final load belongs to the already-admitted gap. The shared helper makes three attempts, accepts recovery from transient errors, stops immediately on a witnessed change and sleeps only between attempts. Its five-second contexts do not bound mutex waiting or filesystem `Stat`. |
| +- **Registry generation:** the four triggers cover insert/delete, changed values of the six columns currently consumed by discovery/attribution, and rowid immutability (`registry.go:569`, `:602`, `:612`). Older-build raw DML still fires them. The rowid trigger blocks the `UPDATE OR REPLACE` deletion escape. Unrelated updates and bookkeeping do not bump the generation; database-less route insertions/deletions still do, so “nothing else” needs that qualification. |
| +- **Replacement chain:** `chainClosed` checks each predecessor’s own closure before trusting its latch (`litestream_metered.go:417`). A successor closed unopened over a live latched predecessor remains linked. Window publication, detachment and the latch flag occur under `staleMu`; production reads of the published chain use that lock. Within the documented single chain, no replacement opens beside an open or draining predecessor. Own and inherited windows are preserved, and inheritance is delivered once per fleet. |
| +- **Close’s last look:** the order is store stopped → watch stopped → file look → witness → inheritance → closed (`litestream_metered.go:724`). A persistent changed or vanished owned file on a current fleet is now held. Already-stale fleets skip both checks. A legitimate rename after the last replication activity can conservatively hold every recorder. The full tree walk has no deadline or breadth bound; it can delay Close, and filesystem stalls are not bounded by the earlier store-close context. |
| +- **Lineage:** `ownedReplicaKey` uses account plus cleaned pathname for both replica and shadow state (`litestream_metered.go:910`). Different valid account IDs do not share keys, including after rename. A→B→A at the same pathname resumes A’s previous lineage. |
| +- **Leading `..`:** the rule accepts the leading-run shape used by the systemd fixture and rejects a `..` after a named component (`litestream_owners.go:599`). On stable, link-free ancestry it matches kernel traversal. It does not repair replaceable ancestry or pathname races. |
| +- **Open-item scope:** Litestream exposure includes every relevant reopen and WAL open, not merely initialization. Ancestry includes claim/link paths as well as `dataDir`. Process-exit loss includes routine restart. Same-filesystem bind mounts evade device checks when only one alias is inventoried; two listed aliases are rejected by identity. External-inventory comparison remains absent. |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Fixed” hibernation conflicts — dispositions `:218`:** only exact unit-file/platform paths are covered. Unit-tree children and conflicting active exec claims remain silent. |
| +- **Every non-hibernating claim is accounted for — dispositions `:217`:** an overlapping hibernating claim can still suppress it. |
| +- **An exec file’s directory is the activator’s — `litestream_owners.go:176`, `:233`:** the new explicit enumeration accepts arbitrary absolute paths without establishing that fact. |
| +- **Nothing capable of holding a database is silently passed over — `litestream_owners.go:488`:** unrecognized-name links to regular databases contradict this, despite the subsequent exclusion list. |
| +- **Close holds only on the last witness reading or inheritance — `litestream_metered.go:683`:** the new file look can hold with an unchanged witness. The test comment at `litestream_metered_test.go:686` still says closing a current fleet holds nobody’s day. |
| +- **Every bucket request was counted — `litestream_metered_test.go:979`:** the following assertion establishes only that the server has no requests in flight. |
| +- **Ownership “by its resolved path” — `litestream_owners.go:337`:** inventory retains the route’s spelled pathname at `:190`; only the checked file facts are resolved. The corresponding test comments have been corrected. |
| +- **Reviewer corrections are all complete — dispositions `:223`:** the remaining strict-walk and Close claims prevent that blanket conclusion. Author-reported package/race success is neither confirmed nor contradicted by this source-only review. |
| + |
| +**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 2603f7ca6..1d30686c5 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -138,7 +138,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review5.md`, `CODEX-FINDINGS-metered-re |
| | 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`. [Review eight: untrue from the eighth build to the ninth for a predecessor that had latched; see "Review eight".] | `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. [Review nine: the inventory keeps the path the route spells; the facts checked are the resolved file's.] 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-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. [Reviews nine and ten: the inventory keeps the path the route spells; the facts checked are the resolved file's; where this row says "resolved path" below, read that.] 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 | |
| @@ -214,12 +214,30 @@ Reports: `OPUS-REVIEW-metered-replicator-review9.md`, `CODEX-FINDINGS-metered-re |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Opus P2-1: `Close` took no last look at the files. | Fixed: `closeNow` takes the look (`lookNow`, the body of `lookAgain`) after the store and the watch have stopped and before the witness is read, so a changed or vanished owned file holds from the last clean look. | `litestream_metered.go` `closeNow`, `lookNow` | `TestCloseLooksAtTheFilesOneLastTime` (a swap after the last clean look, then `Close` with no look between: held from that look less the margin, on the recorders); fails with the look removed (mutation run). | |
| -| Astra 5 (P2), carried since review seven: an exec route's database outside the globs was in no list. | Fixed, in strict mode (the platform's replicator): every non-hibernating route's absolute `DBPath` is a claim the inventory looks at, owned when the file is there and said (skipped or a problem) when it is not; a relative one is a problem. The external replicator's config keeps to the globs. | `litestream.go` | `TestExplicitClaimsOutsideTheGlobsAreInventoried`; fails with the explicit claims not appended (mutation run). | |
| -| Opus P3-2: a hibernating route silently dropped a listed path equal to its `DBPath`, a unit claim or the platform's file included. | Fixed: in strict mode such a path is a problem (two claims on one path). | `litestream.go` | the same test (a hibernating route over a unit route's path); fails with the report removed (mutation run). | |
| +| Astra 5 (P2), carried since review seven: an exec route's database outside the globs was in no list. | Fixed in part, in strict mode (the platform's replicator): every non-hibernating route's absolute `DBPath` is a claim the inventory looks at, owned when the file is there and said (skipped or a problem) when it is not; a relative one is a problem. The external replicator's config keeps to the globs. [Review ten, both reviewers: the tenth build checked such a claim as a named file, with nothing asked of its directory, so a tenant's directory link led to another tenant's file; the eleventh build checks it as a tenant's file. A hibernating route over it was still silent; the eleventh build reports it.] | `litestream.go` | `TestExplicitClaimsOutsideTheGlobsAreInventoried`; fails with the explicit claims not appended (mutation run). | |
| +| Opus P3-2: a hibernating route silently dropped a listed path equal to its `DBPath`, a unit claim or the platform's file included. | Fixed for a unit route naming the file and for the platform's files; a unit tree above the file and a non-hibernating exec route were still silent [review ten, Astra 6 and Opus P3-1; the eleventh build reports any hibernating path that the platform or a route that is not hibernating also claims]. | `litestream.go` | the same test (a hibernating route over a unit route's path); fails with the report removed (mutation run). | |
| | Opus P3-3: a routine restart drops the inherited window, not only a crash. | Stated: any process exit; the durability ledger's box-wide incompleteness is what covers it, which the fleet does not rely on. | `litestream_metered.go` `MeteredFleetStore` doc | | |
| | Opus P3-4, Astra 8: nothing pinned `inheritHold` inside `closeNow`; a witnessed change's read count was not asserted; two bare `MkdirAll`; the latch's re-check under the lock untested. | Fixed in part: `TestCloseOfAnUnopenedFleetLatchesItsChain` (a fleet built over a stale, closed one and closed unopened: latched inside `Close`, its recorders told); the retry table asserts the read count for a witnessed change (the errors before it and the one that saw it, no more); the two `MkdirAll` are `mkdirs`. Still open: the re-check under the lock (two successors racing, which the caller forbids); the 300 ms negative check; `withOwners` by base name; retained handles and worker exit; the watch's own call of the look; per-database attribution; the shadow-state directory check in `prepareShadowState` (refused earlier by the inventory's `platformDirectory` check, so untested on its own). | tests | | |
| | Opus P3-1, Astra 7: root-run units and platform-uid tenants. | Open, with approval H; the review-three row now says what the climb does. | | | |
| | Astra 1 to 4, 6 (P1s and the device open): litestream's opens by path; tenant-replaceable ancestry above protected endpoints (`dataDir`, shadow state, a platform link's target); a delayed admitted request past the margin; the crash gap; the header open through a swapped ancestor. | Open, as before, each stated in the code where it applies; the walk by directory descriptor and durable transition state are the fixes and are required before wiring. | | | |
| -| Claims (both): `registry.go` "and on none that did" (now "misses none that did"); `ErrInventoryChanged` "one has gone"; `lookAgain` "every look until the rebuild"; "passes nothing over in silence"; "by its resolved path" (tests); the sidecar test's comment; dispositions rows (the other platforms' open; the climb's stop; "every fixture directory"; "the latch under the lock is the once"; the bind-mount sentence). | Corrected in place. | | | |
| +| Claims (both): `registry.go` "and on none that did" (now "misses none that did"); `ErrInventoryChanged` "one has gone"; `lookAgain` "every look until the rebuild"; "passes nothing over in silence"; "by its resolved path" (tests); the sidecar test's comment; dispositions rows (the other platforms' open; the climb's stop; "every fixture directory"; "the latch under the lock is the once"; the bind-mount sentence). | Corrected in place, bar two review ten found: the strict-walk promise and the `Close` doc, corrected in the eleventh build. | | | |
| Package tests pass under `/tmp` and `~/.cache` (the author's runs). The full gate on this tip is next; the tenth build has not been read by a reviewer. |
| + |
| +## Review ten (reviewed `9301ed62`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-metered-replicator-review10.md`, `CODEX-FINDINGS-metered-replicator-review10.md` (in the tree from the eleventh build's commit). Both found review nine's P2 closed with a test that fails without it, and both found the same fault in the tenth build's own addition: an exec route's database outside the globs was checked as a named file (nothing asked of its directory, directory links followed), so a tenant's directory link led to another tenant's file (Opus P2-1, Astra 1 as P1). Both found the hibernation report partial (a unit tree above the file, a non-hibernating exec route). The eleventh build: |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Opus P2-1, Astra 1 (P1): an explicit exec claim outside the activator's directories was a `namedFile`. | Fixed: a file is checked by the strictest claim on it (`strictestClaim`): a unit route naming it, then any route naming a file outside `dataDir/*.db` and `dataDir/instances/*/*.db` (`activatorFile`), as a `tenantFile` (links resolved by the platform's rules, the chain climbed, the platform's directory above it, the file's uid the chain's); then a unit tree; and only a file in the activator's own directories is a `namedFile`. The first claim by host no longer decides the kind. | `litestream_owners.go` `strictestClaim`, `activatorFile` | `TestExplicitClaimOutsideTheGlobsIsCheckedAsATenantsFile` (owned in the tenant's own chain; refused through the tenant's directory link to another user's directory; refused when named directly in another user's directory); fails with the outside-glob claim checked as a named file (mutation run). | |
| +| Opus P3-1, Astra 6 (P2): a hibernating route over a non-hibernating exec claim, or over a file in a unit tree, was silent. | Fixed: a hibernating path that the platform or any route that is not hibernating also claims (naming it, or a unit tree above it, inside the globs or out) is a problem. | `litestream.go` `liveClaim` | `TestExplicitClaimsOutsideTheGlobsAreInventoried` (a unit file, a unit tree's child, an exec claim outside the globs, each under a hibernating route); fails with the report back to unit files alone (mutation run). | |
| +| Opus P3-2: a route claiming the platform's own database was silently platform-owned. | Fixed: such a path is unowned ("also claimed by the route X"), which refuses the store. | `litestream_owners.go` `routeClaiming` | the same test (a route over the registry); fails with the check removed (mutation run). | |
| +| Opus P3-3: `Close`'s look ran the whole inventory under the lifecycle lock; an unopened fleet's hold reached back to the build; an inventory error held nothing. | Fixed for the last: a look that cannot be taken at Close holds from the last clean look. The other two are stated in the code: Close takes as long as a look, and a fleet never opened holds from its build. | `litestream_metered.go` `lookNow(final)` | `TestCloseHoldsWhenItsLastLookCannotBeTaken`; fails with the error left to drift (mutation run). | |
| +| Opus P3-4, Astra 10: nothing pinned the latch before the closed flag; no negative explicit-claim case; `O_NOFOLLOW` not told from the identity check; the opened-store test's "every request counted" comment. | Fixed in part: a recorder that notes the closed flag when it is told a day pins the order; the explicit-claim test above has two refusals; `TestOpenRegularNoFollowRefusesALinkToTheSameFile` (a link put in the file's place that leads to the very file looked at: `O_NOFOLLOW` refuses it, the identity check would not); the comment says what the assertion establishes. Each of the three fails with its fix removed (mutation runs: the latch after the flag, `O_NOFOLLOW` dropped). Still open, as stated: the re-check under the lock, the 300 ms check, `withOwners` by base name, retained handles and worker exit, the watch's own call of the look, per-database attribution, the shadow-state directory check. | tests | | |
| +| Astra 8 (P2): a link in a unit tree under a name that is not a database's, leading to a regular file, was ignored; empty tree files, entries vanishing under the look, and an absent console database that cannot be told from one never configured. | Fixed for the first: such a link is reported in skipped, so the caller must name it (`Accept`) before the store builds; never followed. The other three are stated in `strictTreeDBs`'s comment as what it passes over on purpose (an empty file holds nothing; an entry that vanishes is churn; the console file is optional on a box without a console), and the comparison against the external replicator's inventory stays a cutover requirement. | `litestream_owners.go` `strictTreeDBs` | `TestStrictTreeNamesALinkToAFile`; fails with the case removed (mutation run). | |
| +| Opus P3-1 of review nine, Astra 9: root-run units and platform-uid tenants; a route naming another user's otherwise valid tree. | Open, with approval H. | | | |
| +| Astra 2, 3, 4, 5, 7 (P1s and the device open): litestream's opens by path; tenant-replaceable ancestry; a delayed admitted request past the margin; the crash and restart gap; the header open through a swapped ancestor. | Open, as before; each stated where it applies. The walk by directory descriptor and durable transition state are the fixes and are required before wiring. | | | |
| +| Claims (both): the `Close` doc (the look can hold too); the named-file docs; "inventories it like any other claim"; "by its resolved path"; the strict-walk promise; the "every request counted" comment; this file's rows for review nine's Astra 5 and Opus P3-2 and its claims row. | Corrected in place. | | | |
| + |
| +Package tests pass under `/tmp` and `~/.cache` (the author's runs). The full gate on this tip is next; the eleventh build has not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review10.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review10.md| new file mode 100644 |
| index 000000000..bfc5571e4 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review10.md |
| @@ -0,0 +1,113 @@ |
| +# Tenth review: feat/carloku-metered-replicator-20261001 at `9301ed62` |
| + |
| +**Verdict: accept with fixes.** Review nine's P2 (Close took no last look at the files) is closed, and its test fails without the fix. One new P2: the new explicit exec claims are checked with the weakest kind of check (no directory, ancestry or owner check), so they open a cross-tenant read that the unit checks would refuse. |
| + |
| +I read the code with `git show 9301ed62:` and ran nothing. The tenth build changes `closeNow`/`lookNow`, `fleetDBsFromRoutes` (explicit claims and the hibernating report), and comments and tests. `checkOwnedFile`, the chain walk, `admit` and the registry triggers are unchanged since `142ef5d0`; only one registry comment changed. |
| + |
| +## Review nine, finding by finding |
| + |
| +| Finding | Now | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1:** Close took no last look at the files | **Closed.** `closeNow` runs `lookNow` after the store and the watch have stopped and before the witness read (`litestream_metered.go:747-749`). | **No.** Without the look, `TestCloseLooksAtTheFilesOneLastTime` reads an unchanged registry and holds nothing. Its recorder loop is not vacuous: recorders are made at build (`:881`), for `a.example.com.db` and `registry.db`. | |
| +| **Opus P3-1:** root-run units, platform-uid tenants | **Open** (approval H). Unchanged; see A and J. | n/a | |
| +| **Opus P3-2:** a hibernating route hid a listed path | **Closed for a unit or platform path** (`litestream.go:544-546`). **Not closed for an exec claim** (P3-1 below). | **No** for what it covers. | |
| +| **Opus P3-3:** a restart drops the inherited window | **Stated correctly** (`:792-800`). | n/a | |
| +| **Opus P3-4:** tests | **Partly closed.** | Retry count after a witnessed change: **no**, it distinguishes (answers repeat the last entry, so an extra read makes `calls > len`). `inheritHold` inside `closeNow`: **no** (the `link()` check after Close). **Yes** for the order "latched before it reads as closed": swapping `:757` and `:758` passes. The re-check under the lock, the 300 ms check, `withOwners` keyed by base name and the `prepareShadowState` directory check are still open, as stated. | |
| +| **Opus claims** | Registry sentence fixed (`registry.go:584`). The strict-walk doc is qualified (`litestream_owners.go:488-491`). The other-platforms row is fixed. The climb's stop is now in the review-three row. | n/a | |
| +| **Astra 1:** litestream's opens by path | **Open**, stated. | yes | |
| +| **Astra 2:** replaceable ancestry above protected endpoints | **Open**, stated. | yes | |
| +| **Astra 3:** delayed admitted request past the margin | **Open**, stated. | n/a | |
| +| **Astra 4:** the crash gap | **Open**, now stated as any process exit. | n/a | |
| +| **Astra 5:** exec claim outside the globs | **Built**, with two gaps: it is checked as `namedFile` (P2-1 below), and a hibernating route over it is silent (P3-1). | **No:** the new test fails without the append. It has no negative case. | |
| +| **Astra 6:** header open through a swapped ancestor | **Open**, stated (`sqliteopen_unix.go:11-20`). | yes | |
| +| **Astra 7:** ownership inferred from the file | **Open.** Larger now for explicit exec claims (P2-1). | n/a | |
| +| **Astra 8:** tests | **Partly closed**, as for Opus P3-4. Fixed fake uids, base-name keying and the timing-based negative checks remain. | see above | |
| +| **Astra claims** | Corrected: "every look", "one has gone", the sidecar comment, `MkdirAll`, the bind-mount sentence. **"By its resolved path" is not fully corrected** (see the claims section). | n/a | |
| + |
| +## Findings |
| + |
| +### P2-1. Explicit exec claims outside the globs are checked as `namedFile`: no directory, ancestry or owner check, and directory links are followed (N, J, A) |
| + |
| +- **Where:** |
| + - `litestream.go:389-394` and `:453-458` now put each non-hibernating exec `DBPath` into the inventory. |
| + - `litestream_owners.go:178-185` gives it `kind = namedFile`. |
| + - `checkOwnedFile` has no `namedFile` case. `os.Stat(root)` at `:282` follows links, and the walk at `:413-430` only `Lstat`s the last component, so every directory link in the path is resolved silently. |
| + - The device comparison is against the resolved root, so it does not catch a redirect onto another filesystem. |
| + - There is no file-uid check (`:438` covers only the tenant kinds). |
| + - The `namedFile` rationale (`:233-235`, "the activator, not a tenant, puts files in it") holds only for `dataDir/*.db` and `dataDir/instances/*/*.db`. Before this build those were the only exec paths that reached it. |
| +- **Scenario:** |
| + 1. The operator adds `--db /srv/shop/data/app.db` for account A (`cli.go:202`). `/srv/shop` is writable by the shop instance's own uid. |
| + 2. The tenant runs `mv data data.real; ln -s /var/lib/private/carlos-app/v.example.com/.cache data`. V keeps an `app.db` in a dot-directory there. `strictTreeDBs` lists that file as *skipped*, so it is not in `dbs`, and the `os.SameFile` comparison at `:133` never sees it. |
| + 3. A's claim is accepted as owned. V's database is streamed into A's lineage and billed to A, and A can restore it. |
| + 4. Other targets work the same way: a hibernating route's database (the activator's, not listed) and any unlisted single-link file. |
| + 5. Where the target *is* listed, the identity comparison makes both entries unowned, so V's own replication drops out. |
| +- **Same root cause:** when a unit route and an exec route of the same account and app both name one file, `singleOwner` picks the first claim by host name. If the exec host sorts first, a unit tenant's file gets `namedFile` instead of `tenantFile` (`:179-183`). |
| +- **Smallest acceptable fix:** |
| + - Use `namedFile` only when `p` matches one of the two `dataDir` globs. Check every other explicit claim as `tenantFile` (resolve links by the platform's rules, climb the chain, require the platform's directory above it, require the file uid to equal the chain's). |
| + - Choose the strictest kind across all claims on a file, not the first claim's. |
| + - Tests: an explicit claim whose directory is a link, and one whose directory belongs to another uid, are both refused. |
| + |
| +### P3-1. A hibernating route over a non-hibernating exec claim still drops it in silence (L, N) |
| + |
| +- **Where:** `litestream.go:538-547`. The report fires only for `unitPaths[p]` or `platformPaths[p]`, not for `explicit[p]` or a globbed exec claim. |
| +- **Scenario:** exec route A (not hibernating) names `/srv/a/live.sqlite`. Route B (hibernating, another account) names the same path. A's file leaves `dbs` with no skip and no problem reported. The same happens inside `dataDir` for a globbed exec claim. |
| +- **Fix:** in strict mode, report any hibernating path that is also claimed by a non-hibernating route. |
| + |
| +### P3-2. A route claiming a platform path is silently platform-owned (L; carried, unreported) |
| + |
| +- **Where:** `litestream_owners.go:142-152`. `platform[p]` wins before claims are read. |
| +- **Scenario:** an exec or unit route names `/data/console/console.db`. It is attributed to ops, and nothing says two claims are on one path. This is the same class as review nine's P3-2. |
| +- **Fix:** in strict mode, report a platform path that any route also claims as a problem. |
| + |
| +### P3-3. Close's look: costs and edges (M) |
| + |
| +- **Where:** `litestream_metered.go:626-656`, `:747`. |
| +- **What happens:** |
| + - It runs the whole inventory (every tenant tree, a header read of every regular file) while holding `life` and `f.mu`. `Current()` callers block for that time. |
| + - On an unopened fleet `lastClean` is still the constructor's instant. A tenant's ordinary deletion between the build and the Close therefore holds every account's days back to the build. |
| + - A `fleetInventory` error sets only `drift`, so Close holds nothing. In practice this is unreachable: only a `filepath.Glob` bad pattern can raise it. |
| + - Hangs through FUSE or a hung mount already block Close through `watching.Wait()`. The new look adds that exposure only to unopened fleets. |
| +- **Fix:** on a look error inside Close, hold from the last clean look (`goStaleFrom(lastCleanLook(), now)`). State the unopened-fleet over-hold. |
| + |
| +### P3-4. Tests (H, O) |
| + |
| +- Nothing pins `inheritHold` before `closed.Store` (`:757-758`), although the test comment says "before the fleet reads as closed". |
| +- No negative test for explicit claims (P2-1). |
| +- Carried and still open: the re-check under the lock, the 300 ms check, base-name `withOwners`, retained handles and worker exit, the watch's own call of the look, per-database attribution, and the `prepareShadowState` directory check. |
| +- The new tests have no wall-clock or temp-root dependence. `-26h` is relative, and both `platformDir`s sit under one root, so `os.Rename` works. |
| +- The review-eight and review-nine reports are in the tree at `9301ed62` (O). |
| + |
| +## The lettered questions (no further finding) |
| + |
| +- **A.** |
| + - A tenant with its own uid cannot get a link or another user's directory into its chain. The checks are `:339`, `:357-368` and `:380`; the top checks at `:385`, `:394` and `:397` run on every accepting path. |
| + - A platform-uid tenant, or any unit when the platform runs as root, stops climbing at once. Its file is accepted wherever its directory and that directory's parent are the platform's (P3-1 of review nine, carried). |
| + - Higher ancestors stay replaceable (Astra 2). |
| +- **B and I.** |
| + - Unchanged: the stale checks at `:211`, `:223` and `:237` are complete up to the return. A look that finds the change after `:237` falls into the admitted-request margin (Astra 3). |
| + - Retries are bounded at 3 × 5 s plus 2 × 200 ms. Only an error that persists, or a witnessed change, counts as a change. |
| + - `closeOnce` holds, and `closed` is set last (`:758`), so no replacement opens beside a draining store. |
| +- **C.** Correct. `host` is the only unique column, so an `UPDATE OR REPLACE` conflict either changes `host` (the update trigger fires) or moves a rowid (refused). The explicit-claim code reads only the six columns. |
| +- **D and J (chain).** |
| + - Look, witness, latch and closed run in order under `f.mu`. The look and the witness hold the same window (both go through `goStaleFrom(lastClean, now)`), so skipping the witness after a stale look loses nothing. |
| + - Every chain read is under `staleMu`. A successor closed unopened over a live latched fleet does not latch. |
| + - The inherited window is applied once. |
| +- **E.** Lineage is keyed by account plus path. A→B→A reuses A's lineage, which is still unstated. |
| +- **F.** As in review nine. Explicit claims that are absent are skipped, a directory is a problem, an empty file is skipped. |
| +- **G.** The open items are stated correctly. Astra 5 is overstated as "fixed" (P2-1, P3-1). |
| +- **J (DBPath).** A unit `DBPath` that is itself a link is refused at `:322`. One whose *directory* is a tenant's link is refused for unit routes and **accepted for exec routes** (P2-1). A platform link into a tenant-replaceable ancestry is Astra 2. |
| +- **K.** Unchanged and correct, with the ancestor caveat stated. |
| +- **L (fixtures).** Unchanged and fine under umask 077 and a temp root reached through a link. |
| +- **M (sequence).** No new unheld day beyond the known class: a swap made and restored between the last clean look and the Close look, including during litestream's final sync in `store.Close`, is unseen (Astra 1). Also known: the delayed admission (Astra 3) and process exit (Astra 4). |
| + |
| +## Claims the code contradicts |
| + |
| +- **`litestream_metered.go:683-688`:** Close "asks the witness one last time" and "holds a day only if that last reading finds a change". The last look can hold too. |
| +- **`litestream_owners.go:176-177` and `:233-235`:** "its directory is the activator's". This is false for explicit claims outside the globs. |
| +- **`litestream.go:386-388`:** "inventories it like any other claim". It gets the weakest check. |
| +- **`litestream_owners.go:336-337` and dispositions row at line 141:** "owned by its resolved path". The row says the opposite one sentence earlier. `fleetInventory` stores the spelled path (`:190`). |
| +- **Dispositions, Astra 5 "Fixed":** see P2-1 and P3-1. |
| +- **`TestCloseOfAnUnopenedFleetLatchesItsChain` comment:** "before the fleet reads as closed" is not asserted. |
| +- **"Package tests pass":** these are the author's runs. This review neither confirms nor contradicts them. |
| + |
| +Fix P2-1 and P3-1 before accepting this build. Astra 1, 2 and 6 (the walk by directory descriptor), Astra 3 and 4, and the platform-uid/root-unit item still block wiring and attestation. |
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go| index 749f3552b..be5edeee8 100644 |
| --- a/internal/ops/litestream.go |
| +++ b/internal/ops/litestream.go |
| @@ -384,8 +384,10 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| } |
| // An exec route's database outside the globs (a path of the |
| // operator's choosing): the platform's own replicator inventories |
| - // it like any other claim, since a route named it; the external |
| - // replicator's config keeps to the globs as it always has. |
| + // it, since a route named it, and checks it as a tenant's named |
| + // file (strictestClaim: nothing is known of who can write in its |
| + // directory); the external replicator's config keeps to the |
| + // globs as it always has. |
| if tree != nil && r.DBPath != "" { |
| if !filepath.IsAbs(r.DBPath) { |
| problems = append(problems, r.Host+": a route with a relative database path ("+r.DBPath+")") |
| @@ -537,12 +539,14 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| } |
| if hibernating[p] { |
| // A hibernating route's database is the activator's to |
| - // replicate, not this fleet's. One that is also a unit route's |
| - // claim or the platform's own file is two claims on one path, |
| - // and the platform's replicator says so rather than letting |
| - // the hibernating one hide the other. |
| - if tree != nil && (unitPaths[p] != "" || platformPaths[p]) { |
| - problems = append(problems, p+": claimed by a hibernating route and by a unit route or the platform") |
| + // replicate, not this fleet's. One that is also the platform's |
| + // own file, or claimed by any route that is not hibernating (a |
| + // unit route naming it or a tree above it, an exec route |
| + // naming it, inside the globs or out) is two claims on one |
| + // path, and the platform's replicator says so rather than |
| + // letting the hibernating one hide the other. |
| + if tree != nil && (platformPaths[p] || liveClaim(routes, p) != "") { |
| + problems = append(problems, p+": claimed by a hibernating route and by the platform or a route that is not hibernating") |
| } |
| continue |
| } |
| @@ -697,3 +701,18 @@ func sortedKeys(m map[string]bool) []string { |
| sort.Strings(keys) |
| return keys |
| } |
| + |
| +// liveClaim is the host of a route that is not hibernating and claims p, |
| +// by naming it or (a unit route) a directory above it, or "". |
| +func liveClaim(routes []registry.Route, p string) string { |
| + for _, r := range routes { |
| + if r.Hibernate || r.DBPath == "" { |
| + continue |
| + } |
| + claim := filepath.Clean(r.DBPath) |
| + if claim == p || (r.Backing == registry.BackingUnit && strings.HasPrefix(p, claim+string(filepath.Separator))) { |
| + return r.Host |
| + } |
| + } |
| + return "" |
| +} |
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go| index 22d1606fe..06dfb873b 100644 |
| --- a/internal/ops/litestream_metered.go |
| +++ b/internal/ops/litestream_metered.go |
| @@ -617,13 +617,16 @@ func (f *MeteredFleet) lookAgain() { |
| if f.closed.Load() || f.stale.Load() { |
| return |
| } |
| - f.lookNow() |
| + f.lookNow(false) |
| } |
| // lookNow is lookAgain without its guard: the look itself, from the |
| // routes this fleet was built over, held from the last clean look when it |
| -// finds a change. |
| -func (f *MeteredFleet) lookNow() { |
| +// finds a change. A look that cannot be taken at all leaves the watch to |
| +// try again (drift) and, at Close (final), where there is no again, holds |
| +// from the last clean look: a fleet that cannot say its files were as it |
| +// left them is in doubt from then. |
| +func (f *MeteredFleet) lookNow(final bool) { |
| began := time.Now() |
| if f.lookDelay > 0 { |
| time.Sleep(f.lookDelay) |
| @@ -631,6 +634,9 @@ func (f *MeteredFleet) lookNow() { |
| inv, err := fleetInventory(f.routes, f.registryPath, f.dataDir) |
| if err != nil { |
| f.drift.Store(true) |
| + if final { |
| + f.goStaleFrom(f.lastCleanLook(), time.Now()) |
| + } |
| return |
| } |
| for path, was := range f.facts { |
| @@ -680,11 +686,12 @@ func (f *MeteredFleet) Databases() int { |
| return len(f.store.DBs()) |
| } |
| -// Close stops replication, then the watch, then asks the witness one last |
| -// time and releases it, in that order. The fleet answers ErrOwnershipStale |
| -// afterwards and cannot be opened again. Closing a fleet that was current |
| -// holds a day only if that last reading finds a change, or if the chain |
| -// it replaces has closed with a window to pass on (inheritHold); its last |
| +// Close stops replication, then the watch, then looks at the files and |
| +// asks the witness one last time each, and releases the witness, in that |
| +// order. The fleet answers ErrOwnershipStale afterwards and cannot be |
| +// opened again. Closing a fleet that was current holds a day only if that |
| +// last look or that last reading finds a change, or if the chain it |
| +// replaces has closed with a window to pass on (inheritHold); its last |
| // requests are admitted and counted like any others, and only when the |
| // store has stopped does the gate shut. It runs once: a second caller, |
| // concurrent or later, waits for the first shutdown to finish and gets its |
| @@ -741,11 +748,15 @@ func (f *MeteredFleet) closeNow() error { |
| // files first: the successor's own inventory compares nothing with |
| // this fleet's facts, so a swap it finds in place it refuses and a |
| // swap restored by then it accepts, and only this look holds the days |
| - // litestream may have shipped through it. The reading has the same |
| - // patience as an admission's: one busy read must not hold every |
| - // account's days on this fleet and its replacement. |
| + // litestream may have shipped through it. The look walks every tree |
| + // the fleet was built over, under the lifecycle lock, so Close takes |
| + // as long as a look does; on a fleet never opened its last clean look |
| + // is the constructor's, so a change found now holds back to the |
| + // build. The reading has the same patience as an admission's: one |
| + // busy read must not hold every account's days on this fleet and its |
| + // replacement. |
| if !f.stale.Load() { |
| - f.lookNow() |
| + f.lookNow(true) |
| } |
| if !f.stale.Load() { |
| if ok, err := f.witnessUnchanged(); err != nil || !ok { |
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go| index a7764c01f..5221a41b8 100644 |
| --- a/internal/ops/litestream_metered_test.go |
| +++ b/internal/ops/litestream_metered_test.go |
| @@ -976,7 +976,9 @@ func TestMeteredFleetOpenedReplicatesCountsAndStops(t *testing.T) { |
| t.Fatalf("%s was told its day is incomplete while nothing had changed", path) |
| } |
| } |
| - // Every request at the bucket was counted by somebody. |
| + // No request is still in flight at the bucket (what the counts below |
| + // are compared against; that each was counted to its own database's |
| + // recorder is not asserted here). |
| eventually(t, "the counts to settle", func() bool { return bucket.inFlight.Load() == 0 }) |
| // The shadow state is still the platform's. |
| root := filepath.Join(dir, ".replicator") |
| @@ -1744,10 +1746,30 @@ func TestCloseLooksAtTheFilesOneLastTime(t *testing.T) { |
| } |
| } |
| +// closedWhenHeld is a recorder that notes, each time it is told a day is |
| +// incomplete, whether the fleet it belongs to already read as closed. |
| +type closedWhenHeld struct { |
| + countingRecorder |
| + fleet **MeteredFleet |
| + seen []bool |
| +} |
| + |
| +func (c *closedWhenHeld) Incomplete(at time.Time) { |
| + c.countingRecorder.Incomplete(at) |
| + c.mu.Lock() |
| + defer c.mu.Unlock() |
| + if *c.fleet != nil { |
| + c.seen = append(c.seen, (*c.fleet).closed.Load()) |
| + } |
| +} |
| + |
| // Closing a fleet that was never opened still latches its chain, inside |
| // Close, before the fleet reads as closed: the fleet's own recorders are |
| -// told its predecessor's days. (Review nine, Opus P3-4: the chain tests |
| -// drove inheritHold by hand and pinned nothing in closeNow.) |
| +// told its predecessor's days while the fleet is still open to a |
| +// replacement's eyes, so the window is never read from a fleet that is |
| +// closed and not yet latched. (Review nine, Opus P3-4, and review ten: |
| +// the chain tests drove inheritHold by hand and pinned nothing in |
| +// closeNow, nor its order.) |
| func TestCloseOfAnUnopenedFleetLatchesItsChain(t *testing.T) { |
| newFakeBucket(t) |
| dir := platformDir(t) |
| @@ -1766,9 +1788,10 @@ func TestCloseOfAnUnopenedFleetLatchesItsChain(t *testing.T) { |
| if err := a.Close(); err != nil { |
| t.Fatal(err) |
| } |
| - recs := &recorders{} |
| - o.Meter, o.Replaces = recs.meter, a |
| - b, err := MeteredFleetStore(o) |
| + var b *MeteredFleet |
| + watching := &closedWhenHeld{fleet: &b} |
| + o.Meter, o.Replaces = func(OwnedDB) lease.RequestRecorder { return watching }, a |
| + b, err = MeteredFleetStore(o) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -1781,9 +1804,48 @@ func TestCloseOfAnUnopenedFleetLatchesItsChain(t *testing.T) { |
| if prev, latched := b.link(); !latched || prev != nil { |
| t.Fatalf("B after Close: latched %v, predecessor %v; want latched and the chain let go", latched, prev) |
| } |
| + if !watching.days()["2026-10-02"] { |
| + t.Fatalf("B's recorder holds %v; want A's day, told at B's Close", watching.days()) |
| + } |
| + for i, closed := range watching.seen { |
| + if closed { |
| + t.Fatalf("hold %d was told after B read as closed; want the latch before the closed flag", i) |
| + } |
| + } |
| +} |
| + |
| +// A look that cannot be taken at Close holds from the last clean look: a |
| +// fleet that cannot say its files were as it left them is in doubt from |
| +// then. (Review ten, Opus P3-3.) |
| +func TestCloseHoldsWhenItsLastLookCannotBeTaken(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) |
| + } |
| + lastClean := time.Now().UTC().Add(-26 * time.Hour) |
| + fleet.lastClean.Store(lastClean.UnixNano()) |
| + fleet.proven.Store(time.Now().UnixNano()) |
| + // A data directory the glob cannot be built over: the only way an |
| + // inventory fails outright. |
| + fleet.dataDir = string([]byte{'['}) |
| + if err := fleet.Close(); err != nil { |
| + t.Fatal(err) |
| + } |
| + from, _, stale := fleet.staleWindow() |
| + if !stale || !from.Equal(lastClean.Add(-ownershipMargin)) { |
| + t.Fatalf("after a Close whose look failed: stale %v from %v; want held from the last clean look %v less the margin", stale, from, lastClean) |
| + } |
| for path, rec := range recs.all() { |
| - if !rec.days()["2026-10-02"] { |
| - t.Fatalf("%s holds %v; want A's day, told at B's Close", path, rec.days()) |
| + if !rec.days()[lastClean.Format("2006-01-02")] { |
| + t.Fatalf("%s holds %v; want the last clean look's day", path, rec.days()) |
| } |
| } |
| } |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index d797b7eea..324484b53 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -140,6 +140,13 @@ func fleetInventory(routes []registry.Route, registryPath, dataDir string) (Flee |
| continue |
| } |
| if own, ok := platform[p]; ok { |
| + // The box's own database, claimed by a route as well: two |
| + // claims on one path, said rather than resolved in the |
| + // platform's favour. |
| + if host := routeClaiming(routes, p); host != "" { |
| + unowned(p, "the platform's own database is also claimed by the route "+host) |
| + continue |
| + } |
| facts, why := checkOwnedFile(filepath.Dir(p), p, platformDirectory) |
| if why != "" { |
| unowned(p, why) |
| @@ -169,19 +176,19 @@ func fleetInventory(routes []registry.Route, registryPath, dataDir string) (Flee |
| unowned(p, "no single account claims it") |
| continue |
| } |
| - // The claim is on a directory the tenant can write in, or on the |
| - // file itself. A unit tenant's file is checked as its tree would |
| - // be, from the file's own directory: the tenant can write there, |
| - // so the directory above must be the platform's and the file the |
| - // tenant's own. An exec route's file is checked as a named file: |
| - // its directory is the activator's. |
| - root, kind := filepath.Clean(owner.DBPath), tenantTree |
| - if root == p { |
| - root, kind = filepath.Dir(p), namedFile |
| - if owner.Backing == registry.BackingUnit { |
| - kind = tenantFile |
| - } |
| - } |
| + // How the file is checked follows the strictest claim on it, not |
| + // the first: a unit route naming the file, then any route naming |
| + // a file outside the activator's directories, then a unit tree, |
| + // then an exec route's file in the activator's directories. A |
| + // unit tenant's file is checked as its tree would be, from the |
| + // file's own directory: the tenant can write there, so the |
| + // directory above must be the platform's and the file the |
| + // tenant's own. A file an exec route names outside the |
| + // activator's directories (dataDir/*.db, dataDir/instances/*/*.db) |
| + // is checked the same way: nothing is known of who can write in |
| + // its directory. Only a file in the activator's directories is a |
| + // named file, whose directory the activator keeps. |
| + root, kind := strictestClaim(claims, p, dataDir) |
| facts, why := checkOwnedFile(root, p, kind) |
| if why != "" { |
| unowned(p, why) |
| @@ -195,6 +202,54 @@ func fleetInventory(routes []registry.Route, registryPath, dataDir string) (Flee |
| return inv, nil |
| } |
| +// routeClaiming is the host of a route whose DBPath is exactly p, or "". |
| +func routeClaiming(routes []registry.Route, p string) string { |
| + for _, r := range routes { |
| + if r.DBPath != "" && filepath.Clean(r.DBPath) == p { |
| + return r.Host |
| + } |
| + } |
| + return "" |
| +} |
| + |
| +// activatorFile reports whether p is a file in the activator's own |
| +// directories (dataDir/*.db or dataDir/instances/*/*.db), the only places |
| +// where an exec route's database is in a directory a tenant cannot write. |
| +func activatorFile(p, dataDir string) bool { |
| + for _, pattern := range []string{filepath.Join(dataDir, "*.db"), filepath.Join(dataDir, "instances", "*", "*.db")} { |
| + if ok, _ := filepath.Match(pattern, p); ok { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| +// strictestClaim is the root and kind the file p is checked with, from the |
| +// routes that claim it (all of one account and app, singleOwner): the |
| +// strictest kind any claim implies. |
| +func strictestClaim(claims []registry.Route, p, dataDir string) (string, claimKind) { |
| + root, kind, rank := "", claimKind(0), -1 |
| + consider := func(r string, k claimKind, n int) { |
| + if n > rank { |
| + root, kind, rank = r, k, n |
| + } |
| + } |
| + for _, c := range claims { |
| + claim := filepath.Clean(c.DBPath) |
| + switch { |
| + case claim == p && c.Backing == registry.BackingUnit: |
| + consider(filepath.Dir(p), tenantFile, 3) |
| + case claim == p && !activatorFile(p, dataDir): |
| + consider(filepath.Dir(p), tenantFile, 2) |
| + case claim != p: |
| + consider(claim, tenantTree, 1) |
| + default: |
| + consider(filepath.Dir(p), namedFile, 0) |
| + } |
| + } |
| + return root, kind |
| +} |
| + |
| // singleOwner picks the route that owns a database from the routes that |
| // claim it. There must be at least one, each must name an account, and all |
| // must name the same account and app. The host is the first claim's by |
| @@ -230,9 +285,10 @@ const ( |
| // could swap its whole tree for a link), and the file must belong to |
| // the user the tree belongs to. |
| tenantTree |
| - // namedFile: an exec route names the file itself. Nothing is asked of |
| - // its directory here: an exec instance's directory is the activator's, |
| - // and the activator, not a tenant, puts files in it. |
| + // namedFile: an exec route names a file in the activator's own |
| + // directories (activatorFile). Nothing is asked of its directory |
| + // here: the activator, not a tenant, puts files in it. A file an exec |
| + // route names anywhere else is a tenantFile. |
| namedFile |
| // tenantFile: a unit route names the file itself. The file's whole |
| // chain of directories, up to where the tenant's ownership stops, must |
| @@ -334,8 +390,9 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| // the link's own directory) 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. |
| + // link at any depth is owned under the path the route spells, |
| + // with the resolved file's facts. A link anywhere else in the |
| + // path, the tenant's or anyone's, is refused. |
| resolved, why := resolvePlatformLinks(root) |
| if why != "" { |
| return fileFacts{}, why |
| @@ -487,7 +544,8 @@ const strictTreeMaxDepth = 32 |
| // deeper than treeDBWalkDepth, outside dot-directories) and, unlike it, |
| // passes nothing over in silence that could hold a database, because a |
| // replicator that has to say every database was counted cannot have |
| -// places it did not look (what it does pass over on purpose is listed at |
| +// places it did not look (the two things it passes over on purpose, an |
| +// entry that vanishes under the look and an empty file, are listed at |
| // the end): |
| // |
| // - a database it would not replicate is reported in skipped: one in a |
| @@ -500,8 +558,11 @@ const strictTreeMaxDepth = 32 |
| // strictTreeMaxDepth, is a problem; |
| // - a link is never followed. One that is named like a database, or that |
| // leads to a directory, is a problem: the tenant's own software would |
| -// follow it, so there may be a database behind it. Any other link is |
| -// ignored. |
| +// follow it, so there may be a database behind it. One that leads to |
| +// a regular file under any other name is reported in skipped, so a |
| +// database the tenant reaches through a link of its own (current -> |
| +// live.sqlite) is named to the caller, never passed over. A link that |
| +// leads nowhere is ignored. |
| // |
| // An entry that vanishes between being listed and being looked at is |
| // ordinary churn (SQLite's journals come and go) and is ignored. An empty |
| @@ -532,8 +593,11 @@ func strictTreeDBs(root string) (dbs, skipped, problems []string) { |
| switch { |
| case fi.Mode()&os.ModeSymlink != 0: |
| target, terr := os.Stat(p) |
| - if sqliteName(name) || (terr == nil && target.IsDir()) { |
| + switch { |
| + case sqliteName(name) || (terr == nil && target.IsDir()): |
| problems = append(problems, p+": a link that may lead to a database") |
| + case terr == nil && target.Mode().IsRegular(): |
| + skipped = append(skipped, p) |
| } |
| case fi.IsDir(): |
| if depth+1 > strictTreeMaxDepth { |
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go| index 13c4ea84e..85e4d3dc4 100644 |
| --- a/internal/ops/litestream_owners_test.go |
| +++ b/internal/ops/litestream_owners_test.go |
| @@ -591,16 +591,53 @@ func TestExplicitClaimsOutsideTheGlobsAreInventoried(t *testing.T) { |
| if !contains(inv.Skipped, missing) && !containsPrefix(inv.Problems, missing) { |
| t.Fatalf("an exec route's database that is not there: skipped %v, problems %v; want it said", inv.Skipped, inv.Problems) |
| } |
| - // Two claims on one path, one of them hibernating. |
| + // Two claims on one path, one of them hibernating: a unit route |
| + // naming the file, a unit route's tree above it, and an exec route |
| + // outside the globs that is not hibernating. |
| unit := writeFile(t, filepath.Join(platformDir(t), "apps", "u", "main.db")) |
| upsert(t, reg, registry.Route{Host: "u.example.com", Account: "bae", App: "three", DBPath: unit, Backing: registry.BackingUnit}) |
| upsert(t, reg, registry.Route{Host: "sleepy.example.com", Account: "baf", App: "four", DBPath: unit, Hibernate: true}) |
| + treeRoot := filepath.Join(platformDir(t), "apps", "v") |
| + inTree := writeFile(t, filepath.Join(treeRoot, "main.db")) |
| + upsert(t, reg, registry.Route{Host: "v.example.com", Account: "bag", App: "five", DBPath: treeRoot, Backing: registry.BackingUnit}) |
| + upsert(t, reg, registry.Route{Host: "dozy.example.com", Account: "bah", App: "six", DBPath: inTree, Hibernate: true}) |
| + upsert(t, reg, registry.Route{Host: "napping.example.com", Account: "bai", App: "seven", DBPath: custom, Hibernate: true}) |
| inv, err = FleetDBOwners(reg, dir) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - if !containsPrefix(inv.Problems, unit) { |
| - t.Fatalf("a hibernating route over a unit route's path: owned %+v, problems %v; want a problem", inv.Owned, inv.Problems) |
| + for _, p := range []string{unit, inTree, custom} { |
| + if !containsPrefix(inv.Problems, p) { |
| + t.Fatalf("a hibernating route over %s: owned %+v, problems %v; want a problem", p, inv.Owned, inv.Problems) |
| + } |
| + } |
| + // A route over the platform's own database: said, not resolved in |
| + // the platform's favour. |
| + upsert(t, reg, registry.Route{Host: "greedy.example.com", Account: "baj", App: "eight", DBPath: filepath.Join(dir, "registry.db")}) |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !containsPrefix(inv.Unowned, filepath.Join(dir, "registry.db")) { |
| + t.Fatalf("a route over the registry: owned %+v, unowned %v; want it unowned", inv.Owned, inv.Unowned) |
| + } |
| +} |
| + |
| +// A link in a unit tree that leads to a regular file under a name that is |
| +// not a database's (current -> live.sqlite) is named to the caller as |
| +// skipped, never followed and never passed over. (Review ten, Astra 8.) |
| +func TestStrictTreeNamesALinkToAFile(t *testing.T) { |
| + root := platformDir(t) |
| + target := writeFile(t, filepath.Join(platformDir(t), "elsewhere", "live.sqlite")) |
| + if err := os.Symlink(target, filepath.Join(root, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(filepath.Join(root, "nowhere"), filepath.Join(root, "dangling")); err != nil { |
| + t.Fatal(err) |
| + } |
| + dbs, skipped, problems := strictTreeDBs(root) |
| + if len(dbs) != 0 || len(problems) != 0 || !contains(skipped, filepath.Join(root, "current")) || contains(skipped, filepath.Join(root, "dangling")) { |
| + t.Fatalf("dbs %v skipped %v problems %v; want the link to a file skipped and the dangling one ignored", dbs, skipped, problems) |
| } |
| } |
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go| index 67ea817b6..539dc4b46 100644 |
| --- a/internal/ops/litestream_owners_unix_test.go |
| +++ b/internal/ops/litestream_owners_unix_test.go |
| @@ -328,3 +328,74 @@ func TestAPlatformLinksRelativeTargetIsWalkedNotCleaned(t *testing.T) { |
| t.Fatalf("the facts are not the lexical file's (%s)", lexical) |
| } |
| } |
| + |
| +// An exec route's database outside the activator's directories is checked |
| +// as a tenant's named file: one whose directory is a link the tenant made, |
| +// and one in another user's directory, are refused; one in the tenant's |
| +// own chain beneath the platform's directory is owned. (Review ten, both |
| +// reviewers: the first build of explicit claims checked them as named |
| +// files, with nothing asked of their directories.) |
| +func TestExplicitClaimOutsideTheGlobsIsCheckedAsATenantsFile(t *testing.T) { |
| + const tenant, other = 44001, 44002 |
| + top := platformDir(t) |
| + mkdirs(t, filepath.Join(top, "srv", "A", "live"), filepath.Join(top, "srv", "B", "live")) |
| + own := writeFile(t, filepath.Join(top, "srv", "A", "live", "main.sqlite")) |
| + theirs := writeFile(t, filepath.Join(top, "srv", "B", "live", "main.sqlite")) |
| + withOwners(t, map[string]uint32{"srv": 0, "A": tenant, "B": other, "live": tenant, "main.sqlite": tenant, "link:live": tenant}) |
| + dir := platformDir(t) |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: own}) |
| + inv, err := FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if o, ok := ownedByPath(inv)[own]; !ok || o.Account != "bac" { |
| + t.Fatalf("the tenant's own file: owned %+v, unowned %v", inv.Owned, inv.Unowned) |
| + } |
| + // The directory swapped for a link to B's: refused, not followed. |
| + if err := os.Rename(filepath.Join(top, "srv", "A", "live"), filepath.Join(top, "srv", "A", "live.real")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(filepath.Join(top, "srv", "B", "live"), filepath.Join(top, "srv", "A", "live")); err != nil { |
| + t.Fatal(err) |
| + } |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[own]; ok || !containsPrefix(inv.Unowned, own) { |
| + t.Fatalf("through the tenant's directory link: owned %+v, unowned %v; want refused", inv.Owned, inv.Unowned) |
| + } |
| + // Named directly in another user's directory: refused. |
| + upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: theirs}) |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[theirs]; ok || !containsPrefix(inv.Unowned, theirs) { |
| + t.Fatalf("another user's file named by an exec route: owned %+v, unowned %v; want refused", inv.Owned, inv.Unowned) |
| + } |
| +} |
| + |
| +// The header open refuses a link in the file's place even when the link |
| +// leads to the very file that was looked at: O_NOFOLLOW is what refuses |
| +// it, not the identity check after the open. (Review ten, Astra 10.) |
| +func TestOpenRegularNoFollowRefusesALinkToTheSameFile(t *testing.T) { |
| + dir := platformDir(t) |
| + path := writeFile(t, filepath.Join(dir, "x")) |
| + looked, err := os.Lstat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Rename(path, filepath.Join(dir, "y")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.Symlink(filepath.Join(dir, "y"), path); err != nil { |
| + t.Fatal(err) |
| + } |
| + f, err := openRegularNoFollow(path, looked) |
| + if err == nil { |
| + f.Close() |
| + t.Fatal("a link to the file that was looked at was opened through") |
| + } |
| +} |