amadan
The replicator's twelfth build: the eleventh review's findings
Every check a file's claims imply runs, and one refusal refuses it (the ranked kinds were not ordered: both reviewers). A link in a unit tree is named with its target and a target that cannot be looked at is a problem; a hibernating unit's file under a hibernating exec claim is a problem; a unit tree above the platform's file is a claim; root and the platform's user are one owner at the end of a tenant chain; an isolated instance's database is checked by its chain. Fixtures name the other user's tree apart. Dispositions "Review eleven" and the reports.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
9 files changed,
+622
−59
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review11.md+183 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+20 −3docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review11.md+141 −0internal/ops/litestream.go+5 −1internal/ops/litestream_metered.go+2 −1internal/ops/litestream_metered_test.go+2 −1internal/ops/litestream_owners.go+110 −44internal/ops/litestream_owners_test.go+63 −2internal/ops/litestream_owners_unix_test.go+96 −7
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review11.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review11.md| new file mode 100644 |
| index 000000000..9e0edd2a3 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review11.md |
| @@ -0,0 +1,183 @@ |
| +| Review-ten finding | Closed at `56031316`? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 — weak checks for explicit exec claims** | **Partly.** The reported outside-glob directory-link case is closed. Selecting one supposedly strictest claim still loses restrictions from other claims; finding 5. | **No** for restoring `namedFile` outside the globs. **Yes** for selecting only the first applicable claimant: the new fixture has one claimant. | |
| +| **Opus P3-1 — hibernation hides an exec claim** | **Yes for the reported non-hibernating exec claim.** Hibernating unit claims remain vulnerable; finding 6. | **No** for removing the added coverage. The remaining unit-hibernation case is absent. | |
| +| **Opus P3-2 — route claims a platform database** | **Only for exact file claims.** A unit-tree claim remains silent; finding 7. | **No** for removing `routeClaiming`. Its fixture does not claim the containing directory. | |
| +| **Opus P3-3 — final-look error and Close costs** | **Error handling closed.** Full-inventory shutdown cost and unopened-fleet over-holding are documented, unchanged behavior. | **No.** `TestCloseHoldsWhenItsLastLookCannotBeTaken` asserts staleness, the exact lower bound, and recorder holds. | |
| +| **Opus P3-4 — weak tests** | **Partly.** Negative explicit claims and inheritance-before-closed now have distinguishing assertions. Other gaps remain; finding 11. | **No** for moving inheritance after the closed flag. **Yes** for several remaining mutations below. | |
| +| **Opus claims** | Mostly corrected. The stale Close-test comment, overbroad strict-walk language, and blanket correction claim remain. | Prose has no behavioral test. | |
| +| **Astra 1 — unchecked explicit exec claims** | **Reported single-claim exploit closed; broader claim-strength issue remains.** | **No** for restoring the outside-glob `namedFile` classification; **yes** for losing another claimant’s restrictions. | |
| +| **Astra 2 — Litestream pathname opens** | **No.** | No regression test forces substitution during the actual opens. | |
| +| **Astra 3 — replaceable ancestry** | **No.** | Existing endpoint tests do not establish protected ancestry. | |
| +| **Astra 4 — delayed admission escapes held days** | **No.** | No test exercises the delayed request across midnight. | |
| +| **Astra 5 — process exit loses doubt** | **No.** | No recovery test establishes durable transition state. | |
| +| **Astra 6 — hibernation conflict omissions** | **Reported active-exec and active-unit-tree cases closed.** Hibernating units still disappear. | **No** for restoring the old exact-unit-only check; the remaining case passes unnoticed. | |
| +| **Astra 7 — header probe can open a device** | **No.** | Final-component tests do not exercise ancestor substitution. | |
| +| **Astra 8 — silent inventory exclusions** | **Partly.** Links resolving to regular files are now skipped explicitly. Failed target lookups and earlier omissions remain. | **No** for removing the new link-to-file case. Lookup failures are untested. | |
| +| **Astra 9 — filesystem UID not bound to route** | **No.** | No test rejects a different route’s internally consistent tree. | |
| +| **Astra 10 — weak tests and environmental assumptions** | **Partly.** The same-inode symlink and closed-flag recorder distinguish their fixes. Other gaps remain. | **No** for removing `O_NOFOLLOW` or reversing inheritance/closed ordering; **yes** for mutations listed below. | |
| +| **Astra claims** | Mostly corrected. Remaining contradictions are listed last. | Author-reported test and mutation results were not independently executed. | |
| + |
| +All locations below are at **`56031316`**. This reviews both `72b4f59a..56031316` and `9301ed62..56031316`. Test counterfactuals are source-based; no files were changed, tests run, or network commands issued. |
| + |
| +1. **P1 — Litestream still opens files independently of the ownership checks.** |
| + |
| + **Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:337`; `third_party/litestream/db.go:1004`, `:1014`, `:1032`, `:1866`. |
| + |
| + Tenant A passes inventory, substitutes B’s database during Litestream initialization, then restores A’s original pathname. Litestream can retain B’s descriptor while subsequent inventories see A’s original inode. Initialization can also change the substituted database’s journal mode and create bookkeeping tables. WAL opens remain exposed separately. |
| + |
| + Close’s final look cannot detect a substitution already restored. The dispositions correctly acknowledge this, including subsequent opens rather than initialization alone. |
| + |
| + **Smallest acceptable fix:** carry pinned directory authority through the actual SQLite, database, and sidecar opens, including reopen paths. Comparing identities after an unrestricted open detects some substitutions but cannot prevent the open’s effects. |
| + |
| +2. **P1 — Protected directories still have unchecked, tenant-replaceable ancestry.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:454`, `:519`, `:711`, `:739`; `internal/ops/litestream_metered.go:944`. |
| + |
| + T owns `/srv/T`. It contains root-owned `bridge`, containing tenant-owned `A`. A platform-made link targets `/srv/T/bridge/A`. The immediate checks pass, but T can rename `bridge` through `/srv/T` and substitute the hierarchy. |
| + |
| + The same construction works above a protected `dataDir`: checking `dataDir` and private shadow directories does not protect their access path. A platform link whose **immediate target parent** is tenant-owned is refused; the surviving case is higher ancestry, including above the link’s own parent. |
| + |
| + **Smallest acceptable fix:** establish and pin a trusted ancestry boundary for claims, authorized links, and shadow state before opening or creating anything beneath them. |
| + |
| +3. **P1 — A delayed admitted request can fall on a day absent from every inherited window.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:237`, `:322`, `:758`; `internal/lease/requestgate.go:58`. |
| + |
| + A concrete UTC sequence: |
| + |
| + - A admits a request on October 2 at **23:58:00**, then 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 retains its predecessor link. |
| + - A’s admitted request resumes at **00:02 on October 3**, is recorded, sent, and completes. |
| + - A closes. Because it is already stale, Close skips both final checks and does not extend the window. |
| + - C replaces B. B’s and C’s recorders inherit A’s window correctly, but **October 3 is absent**. |
| + |
| + The lower bound correctly uses the earlier registry proof and clean file look. This failure is the upper bound, not a remaining chain-latching defect. |
| + |
| + **Smallest acceptable fix:** track admitted attempts through recording/completion and incorporate their doubtful dates before finalizing handover. A larger fixed margin is insufficient. |
| + |
| +4. **P1 — Ownership doubt still disappears across process exit.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:343`, `:376`, `:803`. |
| + |
| + A request passes admission, ownership changes, and the request completes before invalidation. The process exits. Its successor reads the new owner with neither a predecessor nor durable uncertainty requiring the completed request’s day to be held. |
| + |
| + Routine restart also loses inheritance that has not reached the replacement’s recorders. An unfinished ledger row covers a different case. |
| + |
| + **Smallest acceptable fix:** persist ownership/admission uncertainty before affected requests can escape, and recover it before successor coverage is established. |
| + |
| +5. **P2 — `strictestClaim` ranks checks that impose different restrictions.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:230`, `:240`, `:244`, `:495`. |
| + |
| + Consider two routes of the same account and app: |
| + |
| + - A unit route claims `/srv/A`, owned by UID A. |
| + - An exec route explicitly names `/srv/A/bridge/B/main.db`. |
| + - `bridge` is protected and root-owned; directory `B` and `main.db` belong to UID B. |
| + |
| + The unit-tree check rejects the file because B differs from the claimed tree’s UID A. The explicit-file check climbs B’s chain and accepts root-owned `bridge` above it. `strictestClaim` selects that check and discards the tree’s UID restriction, accepting a file the tree claimant would reject. |
| + |
| + Similarly, equal-ranked tree claims retain whichever host sorts first; their roots and restrictions are not combined. The new single-claim test does not establish the advertised property. |
| + |
| + **Smallest acceptable fix:** enforce every applicable claimant’s ownership, root, and filesystem restrictions, or derive a combined check demonstrably preserving them. Add overlapping-claim fixtures; a rank alone is insufficient. |
| + |
| +6. **P2 — A hibernating exec route still silently suppresses a hibernating unit’s database.** |
| + |
| + **Locations:** `internal/ops/litestream.go:360`, `:540`, `:548`, `:709`. |
| + |
| + Unit route U has `BackingUnit`, `Hibernate=true`, and names `main.db`, directly or through its state tree. Hibernating exec route E names that file exactly. |
| + |
| + U is correctly enumerated as a unit: unit replication belongs to this fleet even when the unit hibernates. E sets `hibernating[p]`. But `liveClaim` skips **both** routes because both have `Hibernate=true`; the filter drops the file without adding it to any inventory list. |
| + |
| + This also regresses the previous exact-unit-path conflict report. |
| + |
| + **Smallest acceptable fix:** exclude only hibernating **non-unit** routes when identifying this fleet’s claims. Test both a hibernating unit file and a hibernating unit-tree child overlapped by a hibernating exec claim. |
| + |
| +7. **P2 — Platform-versus-unit-tree conflicts remain silently platform-owned.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:142`, `:146`, `:206`. |
| + |
| + A unit route claims `/data/console`; the platform claims `/data/console/console.db`. Discovery lists the database, but `routeClaiming` checks exact equality only. The platform branch accepts it before the directory-claim logic runs. |
| + |
| + Exact route/file conflicts now refuse correctly. Directory claims still bypass that refusal. |
| + |
| + **Smallest acceptable fix:** use the same file-or-unit-tree coverage predicate for platform conflicts as for ordinary attribution. Test a unit route claiming the console directory. |
| + |
| +8. **P2 — Header inspection can still 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 that device, the privileged open can reach `/dev/watchdog`; the subsequent regular-file and identity checks cannot undo its open-side effects. |
| + |
| + `O_NOFOLLOW` protects the final component, `O_NONBLOCK` addresses FIFO blocking, and `O_NOCTTY` prevents controlling-terminal acquisition. None constrains ancestor resolution. The non-Unix implementation correctly refuses header reads. |
| + |
| + **Smallest acceptable fix:** constrain resolution through pinned directory descriptors before the effects-bearing open, with safe file-type handling. Preserve refusal on unsupported platforms. |
| + |
| +9. **P2 — Failed symlink-target lookups are still silently discarded.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:595`, `:597`, `:599`; `internal/ops/litestream.go:525`. |
| + |
| + A unit tree contains `current -> /mounted/state/live.sqlite`. If resolving the target returns `EACCES` or `EIO`, the link has no recognized database suffix, and neither branch reports it. An inaccessible target is treated like a harmless dangling link. |
| + |
| + Empty tree-only files, entries disappearing before their first look, and an absent configured console database also remain unlisted. There is still no comparison against the external replicator’s inventory. |
| + |
| + **Smallest acceptable fix:** report target-resolution errors other than explicitly accepted absence; distinguish required platform databases from optional ones; require external-inventory coverage reconciliation before cutover. |
| + |
| +10. **P3 — Route identity is still not bound to independently established filesystem ownership.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:377`, `:417`, `:437`, `:495`. |
| + |
| + Route A can directly name B’s otherwise valid directory and file. The inferred file/tree UID is consistently B’s, so the check passes and attribution still selects route A. |
| + |
| + A root-run unit can similarly claim an unrelated root-owned database beneath protected root-owned directories. A platform-UID tenant receives platform directory/link treatment. This is not acceptance literally “from anywhere”: type, link count, permissions, parent, device, and sibling checks still apply. |
| + |
| + **Smallest acceptable fix:** bind tenant routes to independently established UIDs and state roots, with explicit treatment of trusted platform workloads. Deployment approval H is separate from establishing that code invariant. |
| + |
| +11. **P3 — Several tests still fail to establish the protections they describe.** |
| + |
| + **Locations:** `internal/ops/litestream_metered_test.go:687`, `:827`, `:950`, `:1097`, `:1376`; `internal/ops/litestream_owners_unix_test.go:20`, `:338`. |
| + |
| + Material counterfactuals remain: |
| + |
| + - Removing `store.Close` leaves the cleared-pointer and refusing-gate assertions satisfied. Retained database handles and worker exit are not asserted. |
| + - Removing the watch’s call to `lookAgain` leaves direct-call file-look tests intact. |
| + - Swapping database recorders can satisfy the opened-store test’s positive counts. |
| + - Ignoring `lookStart` still satisfies the older lower-bound fixture: its registry proof minus the margin already reaches yesterday. The newer last-clean tests distinguish this defect. |
| + - Removing the admission’s under-lock stale check is not independently distinguished by the post-witness race fixture. |
| + - Removing the latch’s under-lock recheck is not challenged by sequential successor tests; racing successors are outside the documented caller contract. |
| + - Removing `prepareShadowState`’s own data-directory check remains undetected by the fixture rejected earlier during inventory. |
| + - The new outside-glob fixture assigns both `live` directories and both files UID A by basename. It tests a foreign ancestor, not a coherent B-owned tree. |
| + |
| + Relevant fixture modes are restored explicitly under umask `077`, and `platformDir` resolves a linked temp-root spelling. Remaining dependencies include basename substitutions, fixed UIDs `44001/44002`, the 300 ms negative scheduling check, integration timing windows, and wall-clock comparisons. The original climb past the temp root is repaired. |
| + |
| + **Smallest acceptable fix:** add retained shutdown references, watch-driven assertions, per-database attempt correspondence, controlled interleavings, and fixture-scoped identities. Preserve the stronger tests already added. |
| + |
| +The other requested checks hold with these qualifications: |
| + |
| +- **Admission and retries:** `admit` checks stale before locking, under the lock, and after the witness read (`internal/ops/litestream_metered.go:211`, `:223`, `:237`). A file finding during the read refuses admission. A finding after the final load belongs to the already-admitted gap. `witnessUnchanged` makes at most three reads, stops immediately on a witnessed change, and tolerates recovered errors. Its five-second contexts do **not** bound mutex waiting or filesystem `Stat`; “3 × 5 seconds + delays” is not a hard elapsed-time bound. |
| +- **Registry generation:** the four triggers cover inserts, deletes, changed values of the six columns currently consumed by attribution/discovery, and rowid immutability (`internal/registry/registry.go:569`, `:602`, `:612`). Older-build raw DML still fires them. The rowid trigger closes the `UPDATE OR REPLACE` deletion escape. Unrelated updates and bookkeeping do not move the counter. Database-less insertions/deletions do, so literal “nothing else” remains too strong. |
| +- **Replacement chain and Close:** each predecessor’s own closure is checked before trusting its latch (`internal/ops/litestream_metered.go:417`). A successor closed unopened over a live latched fleet remains linked. Window publication, detachment, and latch publication use `staleMu`; production reads of the published chain use that lock. Within the single-chain contract, replacements cannot open beside open or draining predecessors, and inheritance is applied once per fleet. |
| +- **Final look:** the order is store stopped → watch stopped → file look → witness → inheritance → closed (`internal/ops/litestream_metered.go:732`). A persistent changed or vanished owned file on a current fleet is held. Already-stale fleets skip the checks. A harmless rename after replication stops can conservatively hold every account; an unopened fleet can hold back to construction. The full walk can block Close, including on filesystem stalls, without a deadline. |
| +- **Tenant-file chain:** on a stable path with a distinct tenant UID, ordinary tenant links and foreign non-platform starting/stopping directories are refused. Protected platform-owned starting directories are an explicit exception. Every accepting path reaches the top checks at `internal/ops/litestream_owners.go:442` and `:451`. A final-component `DBPath` symlink is refused. Higher ancestry and route-to-UID binding remain findings 2 and 10. |
| +- **Outside-glob compatibility:** a consistently platform-owned file under protected platform directories is accepted; an exec file within the ordinary DynamicUser state-link shape can also pass `tenantFile`. A root/platform-owned file directly under a distinct tenant-owned directory is refused. Mixed root/process ownership can also fail the final UID equality on a non-root run. An exec `DBPath` naming a directory remains a problem; only unit claims are tree claims. |
| +- **Lineage:** replica and shadow keys use account plus cleaned pathname (`internal/ops/litestream_metered.go:921`). Different valid accounts do not share keys, including after a rename. A→B→A at the same path resumes A’s earlier lineage; changing app within the same account does not create another key. |
| +- **Strict walk:** case variants are recognized, though uppercase `.DB` is skipped rather than replicated. Every nonempty regular file not already classified by name receives a header read, including sidecar-shaped names. `sidecarOf` is gone, so a linked base no longer suppresses that read. Relative/missing unit claims, units without `DBPath`, empty explicit claims, and discovered files failing their final look are reported. |
| +- **Link classification:** every non-database-named link resolving to a regular file now requires `Accept`, including ordinary links to text or assets. A link reaching a file through an intermediate directory link is also `Skipped`: classification uses the final `Stat` result. It does not read that target’s contents, but it does follow links for metadata. |
| +- **Leading `..`:** the ordinary systemd `../private/...` shape, including multiple leading ascents, is accepted. A `..` after a named component is refused. On stable, link-free ancestry the accepted traversal matches kernel resolution; it does not solve pathname races. |
| +- **Open-item scope:** same-filesystem bind mounts remain undetected when only one alias is inventoried; two listed aliases are rejected by identity. External inventory comparison and separation from the external replicator’s lineage remain unenforced. Strict-only explicit enumeration does not change the external config generator’s discovery. |
| +- **Reports:** both reviewers’ review-eight, review-nine, and review-ten report files are present at this commit. |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Strictest kind any claim implies”** — `internal/ops/litestream_owners.go:227` and dispositions `:233`: selecting one ranked check can discard another root’s UID or filesystem restriction. |
| +- **Platform conflicts are fixed** — dispositions `:235`: exact claims are covered; unit-tree claims are not. |
| +- **Hibernation conflict coverage is complete** — dispositions `:234`: the non-hibernating cases are covered, but the rule excludes hibernating units that this fleet still owns. |
| +- **Only two deliberate strict-walk omissions** — `internal/ops/litestream_owners.go:547`: dangling links are also deliberately ignored, and target-lookup failures are silently ignored without establishing absence. |
| +- **“A link is never followed”** — `internal/ops/litestream_owners.go:559`: `os.Stat` follows it for classification. “Never traversed for inventory or opened for contents” would describe the behavior. |
| +- **Closing a current fleet holds nobody’s day** — `internal/ops/litestream_metered_test.go:686`: the final look, failed final inventory, witness reading, or inheritance can hold it. |
| +- **Closing always asks the witness once more** — `internal/ops/litestream_metered.go:102`, `:689`: stale fleets skip it, including fleets made stale by the final look. |
| +- **All cited claims were corrected** — dispositions `:241`: the remaining statements above prevent that blanket conclusion. The opened-store counting comment and spelled-versus-resolved pathname comments are corrected. Author-reported successful runs are neither confirmed nor contradicted here. |
| + |
| +**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 1d30686c5..58c48044c 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -230,9 +230,9 @@ Reports: `OPUS-REVIEW-metered-replicator-review10.md`, `CODEX-FINDINGS-metered-r |
| | 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 P2-1, Astra 1 (P1): an explicit exec claim outside the activator's directories was a `namedFile`. | Fixed in the eleventh build by ranking the claims' kinds and keeping one; review eleven (Opus P3-1, Astra 5) showed the kinds are not ordered (a tree check and a file check restrict different things), so the twelfth build runs every distinct check the claims imply and refuses on any (`claimChecks`); `namedFile` is kept for `dataDir/*.db` alone, an isolated instance's directory being the instance's own. | `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, when both spell the path the same way [review eleven: a hibernating unit's own file under a hibernating exec claim was dropped, since the unit was not counted live; the twelfth build counts every unit route; an alias spelled through a platform link is still not compared, open]. | `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; from the twelfth build a unit tree above it counts as a claim too [review eleven, Opus P3-3 and Astra 7]. | `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). | |
| @@ -241,3 +241,20 @@ Reports: `OPUS-REVIEW-metered-replicator-review10.md`, `CODEX-FINDINGS-metered-r |
| | 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. |
| + |
| +## Review eleven (reviewed `56031316`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-metered-replicator-review11.md`, `CODEX-FINDINGS-metered-replicator-review11.md` (in the tree from the twelfth build's commit). Both found review ten's main finding closed with a test that fails without it. Both found the eleventh build's own additions wanting: the ranked claim kinds are not ordered (one check replaced another that restricts different things); the hibernation report counted only exact spellings and dropped a hibernating unit's file; a unit tree above a platform file was silent; and the link-to-file skip, as Opus showed, puts every link tenant software makes (a venv, `node_modules/.bin`) in the refusal path for the whole box's next build, and an accepted link could be re-pointed unseen. The twelfth build: |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Opus P3-1, Astra 5 (P2): `strictestClaim` kept one ranked check. | Fixed: every distinct check the claims imply runs (`claimChecks`: a unit route naming the file or any route naming a file outside `dataDir/*.db`, as a `tenantFile`; a unit tree as a `tenantTree`; an exec file in `dataDir` as a `namedFile`), file checks first, and one refusal refuses the file. | `litestream_owners.go` `claimChecks` | `TestEveryClaimOnAFileMustPass` (the same account's exec claim on a file the file check accepts and the unit tree over it refuses: refused); fails with the loop stopping at the first check (mutation run) | |
| +| Opus P2-1, Astra 9 (P2): a link to a file was skipped by its own name alone, so a re-pointed accepted link read the same; a `Stat` error other than ENOENT was treated as nothing; and every link tenant software makes becomes an unaccepted skip that refuses the whole box's next build. | Fixed for the first two: a link entry is named with its target (`p -> target`), so `Accept` names the pair and a re-pointed link reads as a change; a link whose target cannot be looked at is a problem. The third is stated in `strictTreeDBs`'s comment and here as a decision before wiring: whether one tenant's problems and unaccepted skips refuse the whole box or that tenant's databases alone; today they refuse the box, through `Accept`. | `litestream_owners.go` `strictTreeDBs` | `TestStrictTreeNamesALinkToAFile` (the target named; re-pointed reads differently; a target that cannot be looked at is a problem); each fails with its case removed (mutation runs) | |
| +| Astra 6 (P2): a hibernating exec route over a hibernating unit's file dropped the file (the unit was not counted a live claim). | Fixed: a unit route's database is this fleet's whether or not the unit hibernates, so only a hibernating exec route is no live claim. | `litestream.go` `liveClaim` | `TestExplicitClaimsOutsideTheGlobsAreInventoried` (a hibernating unit under a hibernating exec claim: a problem); fails with units skipped when hibernating (mutation run) | |
| +| Opus P3-3, Astra 7 (P2): a unit tree above a platform file was silently platform-owned; a hibernating path spelled through an alias, or on a skipped file, is unseen. | Fixed for the first: `routeClaiming` uses the inventory's own claim predicate (a unit tree counts). The alias and the skipped-file cases are open: comparing hibernating paths by identity against listed and skipped files is not built. | `litestream_owners.go` `routeClaiming` | the same test (a unit route over the console's directory: unowned); fails with exact spellings alone (mutation run) | |
| +| Opus P3-2: root and the platform's user were two owners at the end of a tenant chain; `instances/*/*.db` rested on a premise isolated exec contradicts; a tenant's file in a platform-owned directory is refused. | Fixed for the first two: root and the platform's user are one owner there as in `platformOwned` (`platformUID`); `activatorFile` is `dataDir/*.db` alone, so an instance's database is checked by its chain. The third is stated as not decided (billable or not), and so refused until it is. | `litestream_owners.go` | `TestRootAndThePlatformUserAreOneOwner`; `TestAnInstancesDatabaseIsCheckedByItsChain` (an instance directory that is a tenant's link: refused); each fails with its fix removed (mutation runs) | |
| +| Opus P3-4, Astra 11: the "other user's directory" fixture keyed by base name was a foreign ancestor, not a coherent tree; the ranking untested. | Fixed: B's directory and file are named apart, and the case now shows what the code does (a coherent other-user tree named by a route is accepted: Astra 9, open with approval H); the every-claim test above. Still open as stated: retained handles and worker exit, the watch's own call, per-database attribution, the re-check under the lock, the 300 ms check, `withOwners` by base name, the admission's under-lock check, the shadow-state directory check. | tests | | |
| +| Astra 1 to 4, 8, 10 (P1s, the device open, the uid binding): 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; a route naming another user's coherent tree. | Open, as before, stated where each applies; required before wiring, with approval H for the last. | | | |
| +| Claims (both): the strict walk's "two things on purpose" and "never followed"; `activatorFile`'s premise; "strictest kind"; the hibernation and platform-conflict rows above; `Close` "asks the witness once more"; the close test's comment; `fleetInventory`'s comment. | Corrected in place. | | | |
| + |
| +Package tests pass under `/tmp` and `~/.cache` (the author's runs). The full gate on this tip is next; the twelfth build has not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review11.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review11.md| new file mode 100644 |
| index 000000000..7db3cafff |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review11.md |
| @@ -0,0 +1,141 @@ |
| +# Eleventh review: feat/carloku-metered-replicator-20261001 at `56031316` |
| + |
| +**Verdict: accept with fixes.** Both reviewers' main finding from review ten is closed, and its test fails without the fix: an exec claim outside the globs is now checked as a `tenantFile`. One new P2 comes from this build's own change to the strict walk. A link to a file is now reported as skipped, so the ordinary links tenant software makes put the whole box's next build in the refusal path. |
| + |
| +I read the source with `git show 56031316:` and ran nothing. Since `9301ed62`, the metered fleet changed only in `lookNow(final)` and comments. The registry is unchanged. The new code is `strictestClaim`, `activatorFile`, `routeClaiming`, `liveClaim` and the link-to-file case in `strictTreeDBs`. |
| + |
| +## Review ten, finding by finding |
| + |
| +| Finding | At `56031316` | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 / Astra 1:** an explicit exec claim was checked as `namedFile` | **Closed** for the reported scenario. A claim outside `dataDir/*.db` and `dataDir/instances/*/*.db` is a `tenantFile` (`litestream_owners.go:242`). Two parts remain: the kinds are not totally ordered (P3-1 below), and the test's "another user's directory" case is not what its fixture builds (P3-4). | **No.** Under `namedFile`, the directory-link case is accepted because `Stat(root)` follows the link and only the leaf is `Lstat`ed. The case of B's directory is also accepted, since `namedFile` makes no owner or parent check. | |
| +| **Opus P3-1 / Astra 6:** a hibernating route over a unit tree or an exec claim was silent | **Closed by spelling** (`litestream.go:548`, `liveClaim`). Aliases and files the walk skipped are still silent (P3-3). | **No.** The `inTree` and `custom` cases fail when the report is narrowed back to unit files. | |
| +| **Opus P3-2:** a route over the platform's database was silent | **Closed for an exact claim** (`litestream_owners.go:146`). Not closed for a unit tree above it (P3-3). | **No.** The `greedy` case fails without `routeClaiming`. | |
| +| **Opus P3-3:** Close's look | **Closed:** a look error at Close holds from the last clean look (`litestream_metered.go:637-639`). The unopened-fleet over-hold and the look running under the lock are stated (`:751-755`). | **No.** Without the branch, the witness is unchanged and nothing is held. The only way to make the look fail is a bad glob pattern, so in production this branch is close to unreachable. That is fine. | |
| +| **Opus P3-4 / Astra 10:** tests | **Closed** for: the latch before the closed flag (`closedWhenHeld`; swapping `:768` and `:769` fails), the two negative explicit-claim cases, `O_NOFOLLOW` (a link to the same inode passes the identity check, so only `O_NOFOLLOW` refuses it), and the "in flight" comment. The other test gaps are still open, as stated. | **No** for each of the four. | |
| +| **Astra 2, 3, 4, 5, 7** (opens by path, replaceable ancestry, delayed admission, crash and restart, the device open through an ancestor) | **Open**, stated where each applies. They are unchanged. | n/a | |
| +| **Astra 8:** silent exclusions in the strict walk | **Closed** for a link to a regular file, which is now skipped. That creates P2-1, and the walk's promise is still not exact (Claims). The comparison against the external replicator's inventory is still open. | **No:** `TestStrictTreeNamesALinkToAFile` fails without the case. | |
| +| **Astra 9:** a unit's uid is inferred from the file, not bound to the route | **Open** (approval H). It is unchanged, and exec claims outside the globs now inherit it: route A naming B's file, in B's own chain, under a platform directory, is accepted as A's. Under `namedFile` it was accepted too, so this is not larger than before. | n/a | |
| +| **Claims (both reviewers)** | The Close doc, the `namedFile` docs, "inventories it like any other claim", "resolved path" and the in-flight comment are corrected. Still wrong: the strict-walk promise, and `activatorFile`'s doc (see Claims). | n/a | |
| + |
| +## Findings |
| + |
| +### P2-1. Any tenant's ordinary links can refuse the next build for the whole box, and an accepted link can be re-pointed without anyone seeing (R, L) |
| + |
| +- **Where:** |
| + - `litestream_owners.go:594-601`: a link to a regular file goes into `skipped`; a link to a directory is a problem. |
| + - `litestream_metered.go:849-864`: any problem, and any skipped entry not named in `Accept`, refuses the store for every account. |
| + - `:659-660`: a new entry only sets drift. |
| +- **Scenario:** |
| + 1. A unit tenant runs `python -m venv venv` in its state directory. That creates `venv/bin/python -> /usr/bin/python3.x`, which is now a skipped entry, and `venv/lib64 -> lib`, which is a problem (this part predates this build). |
| + 2. Or it runs `npm install`, which creates dozens of `node_modules/.bin/*` links to files. The walk descends into dot-directories, so all of them are skipped entries. |
| + 3. The running fleet only drifts. The next rebuild, needed for the next ownership change, refuses with `ErrUnmeterable` until an operator has named every new link. Every tenant's replication on the box stops with it. |
| + 4. Once a link is accepted by path, re-pointing it (`current -> /usr/bin/true`, later `-> live.sqlite`) leaves `render()` unchanged. The new target is never named. |
| +- **How the link is classified:** a link through a directory link to a file is classified by its final target (`os.Stat`), which is right. A failing `Stat` that is not ENOENT (EACCES, ELOOP) is treated as "leads nowhere" and ignored. |
| +- **Smallest acceptable fix:** |
| + - Render a link entry as `link -> target` (from `Readlink`), so a re-pointed link is drift and `Accept` names the pair. |
| + - Report a `Stat` error other than ENOENT as a problem. |
| + - Decide before wiring whether one tenant's problems and unaccepted skips refuse the whole box or only that tenant's databases. Today the cost lands on every account. This must be settled before wiring, not in this build. |
| + |
| +### P3-1. `strictestClaim` ranks kinds that are not ordered, so a second claim from the same account can weaken the check (P) |
| + |
| +- **Where:** `litestream_owners.go:230-251`. |
| + - `tenantFile` (rank 2 or 3) beats `tenantTree` (rank 1). |
| + - A tree claim checks things `tenantFile` does not. Device and component checks run from the tree root, so a mount beneath the tree is refused. The file's uid must equal the tree root's uid, and the directory above the tree root must be the platform's. |
| +- **Scenario:** |
| + 1. Unit route U (account A) claims `/var/lib/carlos-app/u`. The directory is root-owned with mode 0755. |
| + 2. Inside it, `sub` is a mount from another filesystem, owned by uid T, holding `x.db`. |
| + 3. Alone, the tree claim refuses with "crosses onto another filesystem". |
| + 4. The operator adds exec route E for the same account and app naming `.../u/sub/x.db`. Rank 2 wins. The climb stops at `sub` (its parent is root's), the top is `sub`, so the device check is against the mount itself. `platformOwned(u)` passes, the file's uid matches `sub`'s, and the file is accepted. |
| + 5. Two nested unit trees of one account have the same problem: equal rank goes to whichever host sorts first. |
| +- **Bound:** both claims are one account's, so this is a wrong acceptance, not cross-account billing. |
| +- **Smallest acceptable fix:** run `checkOwnedFile` once for each distinct (root, kind) among the claims and require every one to pass. Take the facts from any of them, since all are one inode. |
| + |
| +### P3-2. The new `tenantFile` path refuses some platform layouts, and `activatorFile`'s premise does not hold for isolated exec (P) |
| + |
| +- **Root and the platform's uid count as two owners at the final uid check** (`litestream_owners.go:495`): |
| + - A root-owned file in a directory owned by the platform user (or the reverse) is refused with "does not belong to the user its tenant's directory belongs to". |
| + - `platformDirectory` treats both uids as the platform. |
| + - The case arises on a box where the activator and the replicator run as different platform identities. |
| +- **A tenant-uid file in a platform-owned, non-writable directory is refused** (`:495`). Example: an operator-chosen `/srv/shop/app.db` chowned to the service user in a root-owned 0755 directory. The tenant can write the file's contents but cannot replace the file, so it could be billed to the tenant. |
| +- **A file owned by root or the platform under a tenant-writable directory** gets the right answer. It is refused either at `:424` or at `platformOwned(parent)` (`:454`). |
| +- **A DynamicUser state directory named by an exec route** is handled like a unit's: the platform link is resolved and the chain climbed. |
| +- **`activatorFile`'s doc** (`:215-217`, "a directory a tenant cannot write") is contradicted by `instanceexec/exec.go` at `56031316`: "the directory is the instance's own, so the instance can rewrite it". Under isolated exec, `dataDir/instances/<iid>/<host>.db` is a `namedFile`, with no uid check and no check on its directory. I found no cross-tenant path today, because the instance cannot bring a foreign inode into its own directory. But the premise the kind rests on is false. |
| +- **Smallest acceptable fix:** |
| + - At the final uid check, accept a file uid of 0 or the process's uid when the chain top is the platform's. Decide whether a tenant-uid file in a platform directory that nobody else can write is billable. |
| + - Check `instances/*/*.db` as `tenantFile` too, so only `dataDir/*.db` stays `namedFile`. |
| + |
| +### P3-3. Claims that are still silent (Q) |
| + |
| +- **A unit tree above a platform file:** |
| + - Where: `routeClaiming` (`litestream_owners.go:206`) compares only exact paths. |
| + - Scenario: a unit route with `DBPath = dataDir/console` (or `dataDir`). The tree walk lists `console.db`, the platform branch takes it, and nothing says a route also claims it. |
| +- **A hibernating claim spelled through an alias:** |
| + - Where: `liveClaim` (`litestream.go:705`) compares spellings, and a hibernating path outside the globs never enters `matches` (`:381-383`). |
| + - Scenario: hibernating route B (another account) names `/var/lib/private/carlos-app/u/main.db`, while live unit route U spells the tree `/var/lib/carlos-app/u`. That is two replicators and two accounts on one inode, and nothing is reported. |
| +- **A hibernating claim on a file the walk skipped:** |
| + - Scenario: a hibernating route of another account names `u/.cache/app.db` or `u/main.sqlite`. The file appears only in `Skipped`. The operator accepts it as "not replicated" while the activator streams it under B, and nothing says two routes claim it. |
| +- **Smallest acceptable fix:** |
| + - Use the same claim predicate as `fleetInventory:165-172` (tree prefix included) in `routeClaiming`. |
| + - In strict mode, `Stat` the hibernating paths and compare identities against both listed and skipped files. Tag a skipped entry with any route that names it. |
| + |
| +### P3-4. Tests and environment (H) |
| + |
| +- **`TestExplicitClaimOutsideTheGlobsIsCheckedAsATenantsFile` third case** (`litestream_owners_unix_test.go`, "named directly in another user's directory"): |
| + - `withOwners` is keyed by base name, so `B/live` and `B/live/main.sqlite` take the tenant's uid. Only `B` itself is the other user's. |
| + - The refusal therefore comes from `platformOwned(B)`, not from the file or its directory being another user's. |
| + - A genuinely other user's file in that user's own chain, under a platform directory, is **accepted** (Astra 9). |
| + - The test name and the dispositions row both claim more than this. |
| +- **The ranking itself is untested.** No test has two claims of different kinds on one file, so P3-1 and a "first claim wins" mutation both pass. |
| +- **No wall-clock dependence in the new tests:** the fixed `2026-10-02` comes from `a.goStale(day)`, and `-26h` is relative. |
| +- **The temp root:** `withOwners` fakes `srv` as root and `platformDir` resolves a linked temp root. The new tests carry the same two assumptions as the old ones: the fixed uids 44001/44002, and the test user acting as the platform. |
| +- **Still open, as stated in the dispositions:** |
| + - the re-check under the lock; |
| + - the 300 ms check; |
| + - the base-name key in `withOwners`; |
| + - retained handles and worker exit; |
| + - the watch's own call of the look; |
| + - per-database attribution; |
| + - the `prepareShadowState` check. |
| + |
| +## The lettered questions, briefly |
| + |
| +- **A, J (chain):** unchanged since review ten. |
| + - A tenant with its own uid cannot get a link or another user's directory into its chain. The checks are `:414`, `:424`, `:437` and `:442`, and the top-of-chain link check (`:451`) is reached on every accepting path. |
| + - A tenant whose uid is the platform's, and any unit that runs as root, stops climbing at once. Such a file is accepted wherever its directory is the platform's and nobody else can write it. This is open with approval H. |
| + - The platform-made link: its target's parent must be the platform's (`:739`), and higher ancestry is Astra 2. |
| + - A `DBPath` that is a link to another tenant's file is refused (`:378`). |
| +- **B, I:** the three stale checks (`litestream_metered.go:211`, `:223`, `:237`) are complete up to the return. Retries are bounded, and only an error that persists or a witnessed change counts as a change. `closeOnce` holds and `closed` is set last (`:769`), so no replacement opens beside a draining store. A request admitted before a change can still reach the network after it (Astra 3, open). |
| +- **C:** unchanged and correct, including the rowid trigger and an older build's raw statements. |
| +- **D, J (latch), M:** |
| + - Close runs in this order: store stopped, watch stopped, look, witness, latch, closed. All of it runs under `f.mu`, and `goStaleFrom` takes only `staleMu`, so the order does not deadlock. |
| + - The inherited window is applied once. |
| + - A day in doubt can still go unheld on a recorder in two cases: |
| + - a swap made and restored between the last clean look and Close's look, including during `store.Close`'s final sync (Astra 1); |
| + - Astra 3's delayed admission across midnight. |
| +- **E:** account plus path. A→B→A at one path resumes A's lineage, as review ten said; this is still unstated in the dispositions. |
| +- **F, N:** |
| + - Names are matched case-insensitively. |
| + - Every non-empty regular file is read for the header. |
| + - A directory that cannot be read is a problem. |
| + - A unit route with no `DBPath`, or a relative one, is reported, and an empty claimed file is skipped. |
| + - Explicit claims are now checked as `tenantFile`. That can refuse a file the old check accepted (P3-2), but it accepts nothing the `tenantFile` checks would refuse. The external replicator's config is unchanged. |
| +- **G:** stated correctly. None of the open items is smaller than stated. Astra 9 now also covers exec claims outside the globs. |
| +- **K:** unchanged and correct. `sidecarOf` is gone. Platforms without `O_NOFOLLOW` refuse the header read. |
| +- **L (fixtures):** fine under umask 077 and a temp root reached through a link (see P3-4). |
| +- **O:** the review-eight, nine and ten reports are in the tree. |
| + |
| +## Claims the code contradicts |
| + |
| +- **`litestream_owners.go:547-549`:** the strict walk "passes over on purpose" two things. In fact it also passes over these, and the first three in silence: |
| + - FIFOs, sockets and devices (`:608`); |
| + - links to them; |
| + - links whose `Stat` fails for a reason other than ENOENT; |
| + - dangling links (stated only in the bullet at `:565`). |
| +- **`litestream_owners.go:215-217`, `activatorFile`:** "a directory a tenant cannot write". Under isolated exec, `instances/<iid>` is the instance's own (`instanceexec/exec.go`). |
| +- **Dispositions, review-ten row 1:** "checked by the strictest claim on it" (the kinds are not ordered, P3-1), and "refused when named directly in another user's directory" (the fixture does not build that case, P3-4). |
| +- **Dispositions, review-ten row 2:** "any hibernating path that the platform or a route … also claims" holds only when both spell the path the same way (P3-3). |
| +- **`fleetInventory` comment at `:186-189`:** a file outside the activator's directories is "checked the same way" as a unit tenant's file. It is, but `strictestClaim` can then let that check replace a stricter tree check (P3-1). |
| +- **"Package tests pass":** these are the author's runs, neither confirmed nor contradicted here. |
| + |
| +**Verdict: accept with fixes.** Fix P2-1 (record each link with its target, and treat a `Stat` error other than ENOENT as a problem; decide the box-wide refusal before wiring), P3-1, and the test and claims corrections. Astra 2 to 5 and 7, and approval H's uid binding, still block wiring. |
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go| index be5edeee8..78f7d56da 100644 |
| --- a/internal/ops/litestream.go |
| +++ b/internal/ops/litestream.go |
| @@ -706,7 +706,11 @@ func sortedKeys(m map[string]bool) []string { |
| // 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 == "" { |
| + // A unit route's database is this fleet's whether or not the unit |
| + // hibernates (the activator replicates exec routes' hibernating |
| + // databases, not units'), so only a hibernating exec route is no |
| + // live claim. |
| + if r.DBPath == "" || (r.Hibernate && r.Backing != registry.BackingUnit) { |
| continue |
| } |
| claim := filepath.Clean(r.DBPath) |
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go| index 06dfb873b..5f11b2d31 100644 |
| --- a/internal/ops/litestream_metered.go |
| +++ b/internal/ops/litestream_metered.go |
| @@ -99,7 +99,8 @@ type MeteredFleetOptions struct { |
| Meter func(OwnedDB) lease.RequestRecorder |
| // Replaces is the fleet this one is built to take over from, if any. |
| // The old fleet must be closed before this one is opened, and closing |
| - // it asks the witness one last time. At Open, if that fleet went |
| + // it looks at the files and asks the witness one last time each (a |
| + // fleet already stale skips both). At Open, if that fleet went |
| // stale, every recorder of this one is told the same days are |
| // incomplete: the new owner of a database that changed hands is one |
| // of them, and the old fleet never had its recorder. The caller keeps |
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go| index 5221a41b8..fc6b05ccf 100644 |
| --- a/internal/ops/litestream_metered_test.go |
| +++ b/internal/ops/litestream_metered_test.go |
| @@ -683,7 +683,8 @@ func TestMeteredFleetStoreRefusesAStateDirectoryItCannotRead(t *testing.T) { |
| // Close stops the store before it shuts the gate and lets go of the |
| // witness, and a closed fleet does not open again: the caller builds a new |
| -// one. Closing a fleet that was current holds nobody's day. |
| +// one. Closing a fleet that was current, whose files and registry are as |
| +// it left them, holds nobody's day. |
| func TestMeteredFleetCloseStopsTheStore(t *testing.T) { |
| bucket := newFakeBucket(t) |
| dir := platformDir(t) |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index 324484b53..361dd5774 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -188,10 +188,24 @@ func fleetInventory(routes []registry.Route, registryPath, dataDir string) (Flee |
| // 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) |
| + // Every claim's own check must pass, not the strictest one's |
| + // alone: the kinds are not ordered (a tree claim checks the tree's |
| + // uid and filesystem from its root, a file claim checks the file's |
| + // own chain), so the file is checked once for each distinct root |
| + // and kind the claims imply, and one refusal refuses it. The facts |
| + // are one inode's, whichever check read them. |
| + var facts fileFacts |
| + refused := false |
| + for _, c := range claimChecks(claims, p, dataDir) { |
| + f, why := checkOwnedFile(c.root, p, c.kind) |
| + if why != "" { |
| + unowned(p, why) |
| + refused = true |
| + break |
| + } |
| + facts = f |
| + } |
| + if refused { |
| continue |
| } |
| inv.Owned = append(inv.Owned, OwnedDB{Path: p, Account: owner.Account, App: owner.App, Host: owner.Host, Platform: owner.Account == release.OpsAccount}) |
| @@ -202,52 +216,76 @@ 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 "". |
| +// routeClaiming is the host of a route that claims p, by naming it or (a |
| +// unit route) a directory above it, or "". It is the same predicate the |
| +// inventory uses to gather a file's claims. |
| func routeClaiming(routes []registry.Route, p string) string { |
| for _, r := range routes { |
| - if r.DBPath != "" && filepath.Clean(r.DBPath) == p { |
| + if 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 "" |
| } |
| -// 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. |
| +// activatorFile reports whether p is a file in the activator's own flat |
| +// directory (dataDir/*.db), the one place where an exec route's database |
| +// is in a directory only the activator writes. An isolated instance's |
| +// directory (dataDir/instances/<id>) is the instance's own, so a file |
| +// there is checked as a tenant's file, by its chain. |
| 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 |
| + ok, _ := filepath.Match(filepath.Join(dataDir, "*.db"), p) |
| + return ok |
| +} |
| + |
| +// claimCheck is one check a claim on a file implies: the root it is made |
| +// from and its kind. |
| +type claimCheck struct { |
| + root string |
| + kind claimKind |
| } |
| -// 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 |
| +// claimChecks is every distinct check the routes claiming p imply (all of |
| +// one account and app, singleOwner), in a fixed order: a unit route naming |
| +// the file, or any route naming a file outside the activator's directory, |
| +// is a tenantFile from the file's own directory; a unit route naming a |
| +// directory above it is a tenantTree from that directory; an exec route |
| +// naming a file in the activator's directory is a namedFile. The file must |
| +// pass all of them. |
| +func claimChecks(claims []registry.Route, p, dataDir string) []claimCheck { |
| + seen := map[claimCheck]bool{} |
| + var out []claimCheck |
| + add := func(c claimCheck) { |
| + if !seen[c] { |
| + seen[c] = true |
| + out = append(out, c) |
| } |
| } |
| 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 && (c.Backing == registry.BackingUnit || !activatorFile(p, dataDir)): |
| + add(claimCheck{filepath.Dir(p), tenantFile}) |
| case claim != p: |
| - consider(claim, tenantTree, 1) |
| + add(claimCheck{claim, tenantTree}) |
| default: |
| - consider(filepath.Dir(p), namedFile, 0) |
| + add(claimCheck{filepath.Dir(p), namedFile}) |
| } |
| } |
| - return root, kind |
| + // File checks before tree checks, so a file check's acceptance is |
| + // never the last word when a tree check over it refuses (the order |
| + // changes no answer: every check must pass). |
| + sort.Slice(out, func(i, j int) bool { |
| + if out[i].kind != out[j].kind { |
| + return out[i].kind > out[j].kind |
| + } |
| + return out[i].root < out[j].root |
| + }) |
| + return out |
| } |
| // singleOwner picks the route that owns a database from the routes that |
| @@ -492,7 +530,13 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| if facts.Nlink != 1 { |
| return fileFacts{}, "it has more than one name (a hard link)" |
| } |
| - if (kind == tenantTree || kind == tenantFile) && facts.UID != rootFacts.UID { |
| + // The file's owner must be the chain's, except that root and the |
| + // platform's user are one owner here as they are in platformOwned (a |
| + // root-owned file in the platform user's directory, or the reverse, |
| + // is the platform's either way). A tenant's file in a directory the |
| + // platform owns is refused by this: whether such a file is billable |
| + // to the tenant is not decided, and until it is, it is not. |
| + if (kind == tenantTree || kind == tenantFile) && facts.UID != rootFacts.UID && !(platformUID(facts.UID) && platformUID(rootFacts.UID)) { |
| return fileFacts{}, "it does not belong to the user its tenant's directory belongs to" |
| } |
| for _, suffix := range []string{"-wal", "-shm", "-journal"} { |
| @@ -556,18 +600,27 @@ const strictTreeMaxDepth = 32 |
| // header; an empty file is passed over, it holds nothing); |
| // - a directory that cannot be read, and one deeper than |
| // 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. 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. |
| +// - a link is never walked into or opened; it is classified by where it |
| +// leads (one Stat). 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. One whose target cannot |
| +// be looked at (a permission refused, a dead mount) is a problem too. |
| +// One that leads to a regular file under any other name is reported |
| +// in skipped, with its target, so a database the tenant reaches |
| +// through a link of its own (current -> live.sqlite) is named to the |
| +// caller, never passed over, and a link re-pointed later reads as a |
| +// change. The links tenant software makes in numbers (a venv's, a |
| +// node_modules' .bin) are all named this way: whether one tenant's |
| +// unaccepted links should refuse the whole box's next build, as they |
| +// do today through Accept, or that tenant's databases alone, is to be |
| +// decided before this is wired. |
| // |
| -// 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 |
| -// *.db is a placeholder, as in treeDBs; the fleet's periodic look notices |
| -// when it stops being empty. |
| +// What it passes over on purpose: an entry that vanishes between being |
| +// listed and being looked at (ordinary churn: SQLite's journals come and |
| +// go); an empty regular file (it holds nothing; the fleet's periodic look |
| +// notices when it stops being empty); a link that leads nowhere (there is |
| +// nothing behind it); and FIFOs, sockets and devices, and links to them |
| +// (not files; a database cannot be one). |
| func strictTreeDBs(root string) (dbs, skipped, problems []string) { |
| var walk func(dir string, depth int, hidden bool) |
| walk = func(dir string, depth int, hidden bool) { |
| @@ -592,12 +645,19 @@ func strictTreeDBs(root string) (dbs, skipped, problems []string) { |
| } |
| switch { |
| case fi.Mode()&os.ModeSymlink != 0: |
| + // Classified by where it leads (Stat follows it for that |
| + // and nothing else: it is never walked into or opened). |
| + // A link entry is named with its target, so a link that |
| + // is re-pointed after it was accepted reads as a change. |
| target, terr := os.Stat(p) |
| + dest, _ := os.Readlink(p) |
| switch { |
| case sqliteName(name) || (terr == nil && target.IsDir()): |
| - problems = append(problems, p+": a link that may lead to a database") |
| + problems = append(problems, p+" -> "+dest+": a link that may lead to a database") |
| + case terr != nil && !errors.Is(terr, fs.ErrNotExist): |
| + problems = append(problems, p+" -> "+dest+": a link whose target cannot be looked at") |
| case terr == nil && target.Mode().IsRegular(): |
| - skipped = append(skipped, p) |
| + skipped = append(skipped, p+" -> "+dest) |
| } |
| case fi.IsDir(): |
| if depth+1 > strictTreeMaxDepth { |
| @@ -767,3 +827,9 @@ func sqliteHeader(path string, looked os.FileInfo) (bool, error) { |
| } |
| return n == len(head) && string(head[:]) == "SQLite format 3\x00", nil |
| } |
| + |
| +// platformUID reports whether a uid is the platform's: root, or the user |
| +// this process runs as. |
| +func platformUID(uid uint32) bool { |
| + return uid == 0 || int(uid) == os.Geteuid() |
| +} |
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go| index 85e4d3dc4..a180c239f 100644 |
| --- a/internal/ops/litestream_owners_test.go |
| +++ b/internal/ops/litestream_owners_test.go |
| @@ -621,6 +621,37 @@ func TestExplicitClaimsOutsideTheGlobsAreInventoried(t *testing.T) { |
| 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 unit tree above the platform's own database: the same, by the |
| + // tree (review eleven). |
| + if err := reg.Delete("greedy.example.com"); err != nil { |
| + t.Fatal(err) |
| + } |
| + consoleDir := filepath.Dir(consoleDBPath(dir)) |
| + writeFile(t, consoleDBPath(dir)) |
| + upsert(t, reg, registry.Route{Host: "greedier.example.com", Account: "bak", App: "nine", DBPath: consoleDir, Backing: registry.BackingUnit}) |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !containsPrefix(inv.Unowned, consoleDBPath(dir)) { |
| + t.Fatalf("a unit tree over the console's database: owned %+v, unowned %v; want it unowned", inv.Owned, inv.Unowned) |
| + } |
| + // A hibernating unit's database named by a hibernating exec route: |
| + // the unit's database is this fleet's whether or not it hibernates, |
| + // so the two claims are reported, not dropped (review eleven, Astra 6). |
| + if err := reg.Delete("greedier.example.com"); err != nil { |
| + t.Fatal(err) |
| + } |
| + sleepyUnit := writeFile(t, filepath.Join(platformDir(t), "apps", "w", "main.db")) |
| + upsert(t, reg, registry.Route{Host: "w.example.com", Account: "bal", App: "ten", DBPath: sleepyUnit, Backing: registry.BackingUnit, Hibernate: true}) |
| + upsert(t, reg, registry.Route{Host: "drowsy.example.com", Account: "bam", App: "eleven", DBPath: sleepyUnit, Hibernate: true}) |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !containsPrefix(inv.Problems, sleepyUnit) { |
| + t.Fatalf("a hibernating exec route over a hibernating unit's database: owned %+v, problems %v; want a problem", inv.Owned, inv.Problems) |
| + } |
| } |
| // A link in a unit tree that leads to a regular file under a name that is |
| @@ -636,8 +667,38 @@ func TestStrictTreeNamesALinkToAFile(t *testing.T) { |
| 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) |
| + if len(dbs) != 0 || len(problems) != 0 || !contains(skipped, filepath.Join(root, "current")+" -> "+target) || containsPrefix(skipped, filepath.Join(root, "dangling")) { |
| + t.Fatalf("dbs %v skipped %v problems %v; want the link to a file skipped with its target and the dangling one ignored", dbs, skipped, problems) |
| + } |
| + // Re-pointed, the entry reads differently: an accepted link cannot be |
| + // turned to another file unseen. |
| + if err := os.Remove(filepath.Join(root, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + other := writeFile(t, filepath.Join(platformDir(t), "elsewhere", "other.sqlite")) |
| + if err := os.Symlink(other, filepath.Join(root, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, skipped, _ = strictTreeDBs(root) |
| + if !contains(skipped, filepath.Join(root, "current")+" -> "+other) || contains(skipped, filepath.Join(root, "current")+" -> "+target) { |
| + t.Fatalf("re-pointed: skipped %v; want the new target named", skipped) |
| + } |
| + // A link whose target cannot be looked at is a problem, not nothing. |
| + if os.Geteuid() != 0 { |
| + locked := filepath.Join(platformDir(t), "locked") |
| + mkdirs(t, locked) |
| + inside := writeFile(t, filepath.Join(locked, "live.sqlite")) |
| + if err := os.Chmod(locked, 0o000); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { _ = os.Chmod(locked, 0o755) }) |
| + if err := os.Symlink(inside, filepath.Join(root, "sealed")); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, _, problems = strictTreeDBs(root) |
| + if !containsPrefix(problems, filepath.Join(root, "sealed")) { |
| + t.Fatalf("a link whose target cannot be looked at: problems %v; want it said", problems) |
| + } |
| } |
| } |
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go| index 539dc4b46..32388ad7b 100644 |
| --- a/internal/ops/litestream_owners_unix_test.go |
| +++ b/internal/ops/litestream_owners_unix_test.go |
| @@ -338,10 +338,16 @@ func TestAPlatformLinksRelativeTargetIsWalkedNotCleaned(t *testing.T) { |
| 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")) |
| + // B's directory and file are named apart from A's: withOwners keys by |
| + // base name, and the point is a tree that is coherently B's. |
| + mkdirs(t, filepath.Join(top, "srv", "A", "live"), filepath.Join(top, "srv", "B", "live-b")) |
| 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}) |
| + theirs := writeFile(t, filepath.Join(top, "srv", "B", "live-b", "theirs.sqlite")) |
| + // B also has a main.sqlite, so A's link makes A's spelled path resolve |
| + // to a file that exists; the refusal must come from the link, not from |
| + // the file being missing. |
| + writeFile(t, filepath.Join(top, "srv", "B", "live-b", "main.sqlite")) |
| + withOwners(t, map[string]uint32{"srv": 0, "A": tenant, "B": other, "live": tenant, "main.sqlite": tenant, "link:live": tenant, "live-b": other, "theirs.sqlite": other}) |
| 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}) |
| @@ -356,7 +362,7 @@ func TestExplicitClaimOutsideTheGlobsIsCheckedAsATenantsFile(t *testing.T) { |
| 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 { |
| + if err := os.Symlink(filepath.Join(top, "srv", "B", "live-b"), filepath.Join(top, "srv", "A", "live")); err != nil { |
| t.Fatal(err) |
| } |
| inv, err = FleetDBOwners(reg, dir) |
| @@ -366,14 +372,97 @@ func TestExplicitClaimOutsideTheGlobsIsCheckedAsATenantsFile(t *testing.T) { |
| 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. |
| + // Named directly: another user's file in that user's own coherent |
| + // chain, under the platform's directory, is accepted as the route's |
| + // (nothing binds a route to a uid: review eleven, Astra 9, open with |
| + // approval H); it is refused only where its chain is not one user's. |
| 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) |
| + if _, ok := ownedByPath(inv)[theirs]; !ok { |
| + t.Fatalf("another user's coherent chain named by a route: owned %+v, unowned %v; the uid binding that would refuse this is not built", inv.Owned, inv.Unowned) |
| + } |
| +} |
| + |
| +// Every claim on a file must pass its own check: a unit tree's check (the |
| +// file's uid is the tree's) is not replaced by an exec route's file check |
| +// on the same file, which climbs a shorter chain and would accept it. |
| +// (Review eleven, Opus P3-1 and Astra 5: the eleventh build ranked the |
| +// kinds and kept one.) |
| +func TestEveryClaimOnAFileMustPass(t *testing.T) { |
| + const tenant, other = 44001, 44002 |
| + top := platformDir(t) |
| + mkdirs(t, filepath.Join(top, "srv", "A", "bridge", "B")) |
| + file := writeFile(t, filepath.Join(top, "srv", "A", "bridge", "B", "main.db")) |
| + withOwners(t, map[string]uint32{"srv": 0, "A": tenant, "bridge": 0, "B": other, "main.db": other}) |
| + dir := platformDir(t) |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + // The exec route's file check alone would accept: the climb from B |
| + // stops at root's bridge, and the file is B's as B's directory is. |
| + upsert(t, reg, registry.Route{Host: "e.example.com", Account: "bac", App: "one", DBPath: file}) |
| + inv, err := FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[file]; !ok { |
| + t.Fatalf("fixture: the file check alone refused: %v", inv.Unowned) |
| + } |
| + // The same account's unit tree over A refuses it (the file is not the |
| + // tree's user's), and that refusal stands beside the exec claim. |
| + upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: filepath.Join(top, "srv", "A"), Backing: registry.BackingUnit}) |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[file]; ok || !containsPrefix(inv.Unowned, file) { |
| + t.Fatalf("two claims, one refusing: owned %+v, unowned %v; want refused", inv.Owned, inv.Unowned) |
| + } |
| + // (A tenant's file in a platform-owned directory inside the tenant's |
| + // tree is accepted by both checks: the climb passes through the |
| + // platform's directory into the tenant's parent, and the tree is the |
| + // tenant's. No layout a test can build has the tree accept and the |
| + // file check refuse; the file check runs first so that this case |
| + // tells "every check" from "the first check".) |
| +} |
| + |
| +// An isolated instance's database (dataDir/instances/<id>/<host>.db) is |
| +// checked by its chain, not as a named file: the instance's directory is |
| +// the instance's own, so a link put in its place by the instance is seen. |
| +// (Review eleven, Opus P3-2.) |
| +func TestAnInstancesDatabaseIsCheckedByItsChain(t *testing.T) { |
| + const tenant = 44001 |
| + dir := platformDir(t) |
| + mkdirs(t, filepath.Join(dir, "instances", "elsewhere")) |
| + target := writeFile(t, filepath.Join(dir, "instances", "elsewhere", "app.example.com.db")) |
| + if err := os.Symlink(filepath.Join(dir, "instances", "elsewhere"), filepath.Join(dir, "instances", "i1")); err != nil { |
| + t.Fatal(err) |
| + } |
| + withOwners(t, map[string]uint32{"link:i1": tenant, "elsewhere": tenant, "app.example.com.db": tenant}) |
| + spelled := filepath.Join(dir, "instances", "i1", "app.example.com.db") |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + upsert(t, reg, registry.Route{Host: "app.example.com", Account: "bac", App: "one", DBPath: spelled}) |
| + inv, err := FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[spelled]; ok || !containsPrefix(inv.Unowned, spelled) { |
| + t.Fatalf("an instance directory that is a tenant's link: owned %+v, unowned %v; want refused (target %s)", inv.Owned, inv.Unowned, target) |
| + } |
| +} |
| + |
| +// Root and the platform's user are one owner at the end of a tenant chain, |
| +// as they are in platformOwned: a root-owned file in the platform user's |
| +// directory is the platform's, not a tenant's in someone else's directory. |
| +// (Review eleven, Opus P3-2.) |
| +func TestRootAndThePlatformUserAreOneOwner(t *testing.T) { |
| + top := platformDir(t) |
| + mkdirs(t, filepath.Join(top, "srv", "ops")) |
| + file := writeFile(t, filepath.Join(top, "srv", "ops", "archive.sqlite")) |
| + withOwners(t, map[string]uint32{"srv": 0, "ops": uint32(os.Geteuid()), "archive.sqlite": 0}) |
| + if _, why := checkOwnedFile(filepath.Dir(file), file, tenantFile); why != "" { |
| + t.Fatalf("a root-owned file in the platform user's directory: %q", why) |
| } |
| } |