amadan
The replicator's thirteenth build: the twelfth review's findings
A link entry names its target quoted and Accept compares it as written, so a target that folds or pops under cleaning cannot pass as the accepted pair (both reviewers); the checks on one file must all see one inode; a Readlink error other than ENOENT is a problem; the every-claim test has the case that tells every check from the tree's alone; the root/platform test skips as root; the comments both listed are corrected. Dispositions "Review twelve" and the reports.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
9 files changed,
+554
−66
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review12.md+211 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+20 −5docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review12.md+136 −0internal/ops/litestream.go+6 −4internal/ops/litestream_metered.go+23 −8internal/ops/litestream_metered_test.go+45 −1internal/ops/litestream_owners.go+57 −34internal/ops/litestream_owners_test.go+20 −4internal/ops/litestream_owners_unix_test.go+36 −10
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review12.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review12.md| new file mode 100644 |
| index 000000000..f2ed4ef74 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review12.md |
| @@ -0,0 +1,211 @@ |
| +| Review-eleven finding | Closed at `c8e5e47b`? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 — ordinary links, accepted links changing target, failed target lookups** | **Partly.** Targets are reported and non-ENOENT lookup errors are problems. Box-wide refusal remains. The new target pairing is weakened by `Accept`’s path cleaning; finding 5 below. | **No** if target reporting is removed. **No** for removing lookup-error reporting on a non-root run; **yes** on root, where that case is skipped. **Yes** for the remaining `Accept` defect: the test never constructs a fleet with an accepted pair. | |
| +| **Opus P3-1 — ranking loses another claim’s restrictions** | **Closed for stable paths.** Every distinct `(root, kind)` is checked, including multiple trees. | **No** for restoring the ranking or stopping after the first check in the new fixture. Other combinations—two trees and a unit file overlapping an exec file in `dataDir`—lack distinguishing fixtures. | |
| +| **Opus P3-2 — platform layouts and isolated instances** | **Partly.** Mixed root/process ownership is accepted; isolated-instance files now receive `tenantFile` checks. A tenant-owned file in a protected platform directory can still be refused, explicitly left undecided. | Mixed-UID test: **no on a non-root run; yes on root**, where both fixture UIDs are zero. Instance-link test: **no** if restored to `namedFile`. | |
| +| **Opus P3-3 — silent claims** | **Partly.** A unit tree over the console now conflicts. Hibernating aliases and claims on skipped files remain unreported. | **No** for restoring exact-only platform conflict checks. No test establishes the remaining alias/skipped-file coverage. | |
| +| **Opus P3-4 — tests and environment** | **Partly.** The coherent other-owner fixture and overlapping-claim fixture are present. Shutdown, watch, attribution, interleaving and environment gaps remain. | **No** for the former ranked/first-file-check implementation. **Yes** for several remaining mutations listed in finding 11. The coherent-other-owner case now asserts acceptance, not isolation. | |
| +| **Opus claims** | **Partly corrected.** Several obsolete claims remain beside the new explanations. | Prose has no behavioral test; contradictions are listed below. | |
| +| **Astra 1 — Litestream pathname opens** | **Open.** | No test forces substitution during Litestream’s actual opens. | |
| +| **Astra 2 — replaceable ancestry** | **Open.** | Endpoint fixtures still pass without establishing protected ancestry. | |
| +| **Astra 3 — delayed admission escapes held days** | **Open.** | No test carries an admitted request beyond the margin across midnight. | |
| +| **Astra 4 — process exit loses uncertainty** | **Open.** | No durable-transition/recovery test exists. | |
| +| **Astra 5 — discarded claim restrictions** | **Closed for stable paths.** | **No** for the old ranking or first-check mutation exercised by the new fixture. | |
| +| **Astra 6 — hibernating exec hides hibernating unit** | **Closed by matching pathname.** `liveClaim` now includes hibernating units, including tree claims. | **No** for restoring the old Hibernate filter. The new case directly tests a unit file, not a tree child. | |
| +| **Astra 7 — platform versus unit-tree conflict** | **Closed by matching pathname.** | **No** for restoring exact-only `routeClaiming`. | |
| +| **Astra 8 — header open can reach a device** | **Open.** | Final-component fixtures do not exercise ancestor substitution. | |
| +| **Astra 9 — failed target lookups and silent inventory omissions** | **Partly.** Non-ENOENT target failures are reported. Empty tree-only files, early disappearance, absent configured console databases and external-inventory reconciliation remain. | Lookup-error case: **no on non-root; yes on root**, where it is skipped. The remaining omissions lack closing tests. | |
| +| **Astra 10 — route identity is not bound to filesystem identity** | **Open.** | The corrected coherent-other-owner fixture explicitly accepts the offending layout. | |
| +| **Astra 11 — weak tests** | **Partly.** The misleading fixture and missing overlapping-claim case are corrected. Other gaps remain, with additional qualifications for this build’s tests. | **Yes** for the mutations in finding 11; **no** for the overlapping-claim mutation now exercised. | |
| +| **Astra claims** | **Partly corrected.** | Author-reported runs and mutation results were not independently executed. | |
| + |
| +All locations below refer to **`c8e5e47b`**. I reviewed both requested diffs using committed source. No files were modified, tests run, or network commands issued. |
| + |
| +1. **P1 — Litestream’s actual opens remain outside the ownership checks.** |
| + |
| + **Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:375`; `third_party/litestream/db.go:1004`, `:1014`, `:1032`, `:1866`. |
| + |
| + A tenant passes inventory, substitutes another database while Litestream initializes, and restores the original pathname before the next inventory. Litestream can retain the substituted descriptor while subsequent inventories—including Close’s look—see the original inode. Initialization can also alter the substituted database through SQLite before any later detection. WAL opens remain independently exposed. |
| + |
| + **Smallest acceptable fix:** constrain the actual SQLite, database and sidecar opens through pinned directory authority, including subsequent opens. An identity comparison after an unrestricted open cannot prevent its effects. |
| + |
| +2. **P1 — Protected endpoints still permit tenant-replaceable ancestry.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:492`, `:563`, `:771`, `:799`; `internal/ops/litestream_metered.go:944`. |
| + |
| + Tenant T owns `/srv/T`. Beneath it are root-owned `bridge` and tenant-owned `A`. A platform-made link targets `/srv/T/bridge/A`. The immediate target-parent check accepts `bridge`, but T can rename `bridge` through `/srv/T` and replace the hierarchy. |
| + |
| + The same construction works above a protected `dataDir` and its private shadow directories. A platform-made link whose **immediate target parent** is tenant-owned is refused; the remaining gap is higher ancestry, including above the link’s own parent. |
| + |
| + **Smallest acceptable fix:** establish and pin trusted ancestry for claim roots, permitted platform links and shadow state before performing operations beneath them. |
| + |
| +3. **P1 — A delayed admitted request can still fall outside every inherited hold.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:238`, `:323`, `:759`; `internal/lease/requestgate.go:58`. |
| + |
| + A concrete UTC sequence: |
| + |
| + - Fleet 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. |
| + - A’s request resumes at **00:02 on October 3**, is recorded, sent and completed. |
| + - 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 the window correctly, but **October 3 is absent**. |
| + |
| + `goStaleFrom`’s lower bound and unopened-fleet inheritance are working here. The missing bound is on requests already admitted. |
| + |
| + **Smallest acceptable fix:** track admitted attempts through recording/completion and include their doubtful dates before finalizing the handover. Increasing the fixed margin does not close this. |
| + |
| +4. **P1 — Ownership uncertainty still disappears across process exit.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:344`, `:377`, `:804`. |
| + |
| + A request passes admission, ownership changes, and the request finishes before invalidation. The process exits. Its successor reads the new owner without a predecessor or durable uncertainty covering that completed request. |
| + |
| + A routine restart can also lose inheritance not yet delivered to replacement recorders. An unfinished ledger row covers a different case. |
| + |
| + **Smallest acceptable fix:** persist the relevant ownership/admission uncertainty before affected requests escape, then recover it before establishing successor coverage. |
| + |
| +5. **P2 — The new `link -> target` representation is not a safe acceptance key or inventory encoding.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:653`, `:660`; `internal/ops/litestream_metered.go:675`, `:853`. |
| + |
| + `Accept` applies `filepath.Clean` to the **entire annotated string**. For example, these distinct entries both clean to `/srv/u/live.sqlite`: |
| + |
| + ```text |
| + /srv/u/current -> assets/../live.sqlite |
| + /srv/u/current -> ../../live.sqlite |
| + ``` |
| + |
| + The first link resolves to `/srv/u/live.sqlite`; the second resolves to `/live.sqlite`. Once the first pair is accepted, the second passes a rebuild using the old acceptance list. A running inventory normally detects this textual change, but the required renewed acceptance is bypassed. |
| + |
| + Raw newlines also make `render()` ambiguous. A single skipped entry containing an embedded newline can render identically to two separately sorted entries. For example, one link target containing the literal text: |
| + |
| + ```text |
| + /outside/x |
| + /srv/u/q -> /outside/y |
| + ``` |
| + |
| + can produce the same rendered skipped section as separate `p -> /outside/x` and `q -> /outside/y` entries. Such bytes are permitted in Unix filenames. The unescaped ` -> ` delimiter likewise does not uniquely encode a pair. |
| + |
| + An unchanged ordinary pair remains accepted across rebuilds. Relative targets are displayed verbatim; they are not resolved relative to the link before rendering. |
| + |
| + **Smallest acceptable fix:** represent skipped entries structurally, distinguish files from links, preserve the raw target, and compare unambiguous encoded values. Never path-clean the combined display string. Add fleet-construction and rebuild tests, including relative targets, separators and newlines. |
| + |
| +6. **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` prevents the demonstrated FIFO wait, and `O_NOCTTY` prevents controlling-terminal acquisition. None confines ancestor resolution. |
| + |
| + **Smallest acceptable fix:** constrain resolution through pinned directory descriptors before the effects-bearing open, with safe file-type handling. Preserve refusal on unsupported platforms. |
| + |
| +7. **P2 — Hibernating aliases and claims on skipped files still hide overlapping ownership.** |
| + |
| + **Locations:** `internal/ops/litestream.go:381`, `:479`, `:540`, `:716`; `internal/ops/litestream_owners.go:165`. |
| + |
| + A live unit claims `/var/lib/carlos-app/u`, reached through its platform-made state link. A hibernating exec route belonging to another account names the same database as `/var/lib/private/carlos-app/u/main.db`. The hibernating path never enters the candidate list, and spelling comparisons do not detect the shared inode. |
| + |
| + Similarly, a hibernating route can name `u/.cache/app.db` or `u/main.sqlite`, while the unit-tree walk puts that file only in `Skipped`. Accepting the skipped file does not disclose the competing route. Two hibernating claims are also outside the listed-file conflict checks. |
| + |
| + **Smallest acceptable fix:** reconcile hibernating claims by file identity against listed and skipped files, and attach claimant information to skipped entries before acceptance. |
| + |
| +8. **P2 — Required coverage can still disappear without an inventory entry.** |
| + |
| + **Locations:** `internal/ops/litestream.go:507`, `:525`; `internal/ops/litestream_owners.go:629`, `:639`, `:668`; `internal/ops/litestream_metered.go:846`. |
| + |
| + If `CONSOLE_DB` explicitly names a missing database, it can be dropped exactly like the optional console path on a box without a console. No inventory entry distinguishes those cases. |
| + |
| + Empty tree-only files and entries disappearing before their first inspection remain deliberately omitted. More broadly, a database listed only in the external replicator’s configuration, outside the discovered trees and explicit live claims, never reaches any refusal or acceptance list. |
| + |
| + **Smallest acceptable fix:** distinguish required platform databases from optional ones and require reconciliation against the external replicator’s inventory before cutover. Keep deliberate churn/empty-file exclusions explicit. |
| + |
| +9. **P2 — Ordinary tenant links can still refuse the next fleet for every account.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:655`, `:660`; `internal/ops/litestream_metered.go:850`, `:863`. |
| + |
| + A tenant installs a virtual environment or packages containing `node_modules/.bin` links. Links to regular files require individual acceptance; directory links are unconditional problems. The running fleet reports drift, but its next replacement refuses until those entries are resolved. An unrelated ownership change can therefore leave every account waiting for the tenant’s link inventory to be handled. |
| + |
| + This behavior is now documented; it is not closed. |
| + |
| + **Smallest acceptable fix:** settle and implement the failure scope before wiring—such as isolating the affected tenant’s databases and coverage—or explicitly adopt the box-wide policy with its required recovery procedure. |
| + |
| +10. **P3 — Filesystem ownership remains inferred from the claimed path rather than independently bound to the route.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:174`, `:415`, `:475`, `:539`, `:833`. |
| + |
| + Route A naming B’s coherent directory and file still attributes B’s file to A. The corrected test now explicitly asserts this acceptance at `internal/ops/litestream_owners_unix_test.go:379`. |
| + |
| + The root/process equivalence additionally permits a route to claim a root-owned file in a protected process-owned directory. If the tenant shares that process UID, the new exception does not distinguish its authority from the platform’s. Root-run units similarly pass for unrelated root-owned files under qualifying directories. |
| + |
| + A **distinct non-platform UID** does not gain this exception: both UIDs must be platform UIDs. A root-owned file directly in that tenant’s directory is refused, as is the reverse at the final ownership comparison unless the climb establishes the tenant’s own chain. |
| + |
| + **Smallest acceptable fix:** bind tenant claims to independently established state roots and identities, with explicit treatment of trusted platform workloads. Approval H governs deployment; it does not establish this code invariant. |
| + |
| +11. **P3 — Several tests still permit the failure they claim to exclude.** |
| + |
| + **Locations:** `internal/ops/litestream_metered_test.go:688`, `:828`, `:958`, `:1059`, `:1098`, `:1377`; `internal/ops/litestream_owners_test.go:660`; `internal/ops/litestream_owners_unix_test.go:20`, `:459`. |
| + |
| + Concrete counterfactuals remain: |
| + |
| + - Removing `store.Close` leaves cleared-pointer and refusing-gate assertions satisfied. Retained database handles and worker exit are not asserted. |
| + - Removing the watch’s call to `lookAgain` leaves the direct-call file-look tests intact. |
| + - Swapping the fleet’s database recorders can satisfy the opened-store test’s positive counts. The lower-level lease test does distinguish its two recorders; the fleet integration test does not establish that correspondence. |
| + - Removing admission’s pre-witness, 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 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. |
| + - Ignoring `lookStart` still satisfies the older midnight fixture at `:1098`; the newer last-clean tests distinguish that defect. |
| + - The new link test never exercises acceptance or rebuilding. |
| + - The new root/process test loses its distinguishing UIDs when run as root; the lookup-permission test is skipped there. |
| + |
| + The 300 ms negative check can pass incorrectly if the second goroutine has not run. Integration tests retain scheduling windows and deadlines. The last-clean-start test stores Unix nanoseconds, losing Go’s monotonic-clock component before comparing with wall-clock timestamps, so it remains sensitive to clock adjustment. |
| + |
| + **Smallest acceptable fix:** add retained shutdown references, watch-driven file assertions, per-database attempt correspondence, controlled interleavings, end-to-end acceptance tests and fixture-scoped identities. Make root-specific test limitations explicit. |
| + |
| +The remaining requested checks hold with these qualifications: |
| + |
| +- **Tenant-file chain and platform links:** on a stable path, a distinct tenant UID cannot insert an ordinary tenant-owned link or a foreign non-platform directory into the checked chain. A protected platform-owned starting directory is an explicit exception. Every accepting `tenantFile` path reaches the top checks at `litestream_owners.go:480` and `:489`. A final-component `DBPath` symlink is refused at `:415`. A platform-made link to a target with a tenant-owned immediate parent is refused at `:799`; higher replaceable ancestry remains finding 2. Platform-UID tenants and root-owned files stop climbing immediately at a platform parent, but still face type, permission, device, link-count and sibling checks—acceptance is not literally “from anywhere.” |
| + |
| +- **All claimant checks:** `claimChecks` at `litestream_owners.go:259` retains multiple trees, tree-plus-file checks, and both a unit file check and an exec `namedFile` check inside `dataDir`. No restriction is discarded by ranking. Order does not change acceptance on a stable filesystem. Ordinary DynamicUser state-link shapes and the documented isolated-instance layout can pass. Independent pathname checks do not establish that every successful check saw the same inode during concurrent replacement. |
| + |
| +- **Admission and retries:** stale checks occur before locking, under the lock and after the witness read (`litestream_metered.go:212`, `:224`, `:238`). A file finding during that read refuses admission. A finding after the final stale load belongs to the already-admitted gap. The shared witness helper performs 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`; they are not a hard total elapsed-time bound. |
| + |
| +- **Registry generation:** the four triggers at `registry.go:602` cover inserts, deletes, changed values of all six currently consumed route columns and rowid immutability. Older-build raw statements still fire them. The rowid trigger rejects the `UPDATE OR REPLACE` deletion escape. Unrelated updates, same-value updates and Litestream bookkeeping do not move the counter. Database-less route inserts/deletes do, so literal “nothing else” remains too strong. |
| + |
| +- **Replacement chain and shutdown:** `chainClosed` checks each predecessor’s own closed flag before trusting its latch (`litestream_metered.go:418`). A successor closed unopened over a live latched predecessor remains linked. Window publication, chain detachment and latch publication occur under `staleMu`; published chain reads use that lock. Within the single-chain contract, a replacement cannot open beside an open or draining predecessor. Inheritance is applied once per fleet, including intermediate unopened fleets. It can be read from a stale predecessor intentionally, after closure; it is not prematurely latched from a live predecessor. |
| + |
| +- **Close’s last look:** the order is store stopped → watch stopped → file look → witness → inheritance → closed (`litestream_metered.go:723`). A persistent changed or vanished owned file on a current fleet holds from the last clean look. Already-stale fleets skip the checks. A harmless change after replication stopped can conservatively hold every recorder; an unopened fleet can hold back to construction. The full inventory walk can block Close without a filesystem deadline. |
| + |
| +- **Lineage:** `ownedReplicaKey` at `litestream_metered.go:922` uses account plus pathname. Different valid accounts do not share keys, including after a rename. A→B→A at one path resumes A’s former lineage; changing app within one account does not create a new key. Separation from the external binary’s lineage remains unenforced. |
| + |
| +- **Strict inventory:** names are recognized case-insensitively, although uppercase `.DB` is skipped rather than streamed by tree discovery. Nonempty regular files not already recognized by name receive header reads, including sidecar-shaped names; `sidecarOf` is gone. Unreadable directories, relative/missing unit claims, units without `DBPath`, empty explicit claims and discovered files failing their final look are reported. Explicit exec claims receive the applicable owner checks and do not change the external config generator’s discovery. |
| + |
| +- **Link resolution and fixtures:** a link reaching a file through an intermediate directory link is classified by its final `Stat` result and skipped. The leading-`..` rule permits the ordinary systemd `../private/...` shape, including multiple leading ascents; it rejects ascents after named components. On stable link-free ancestry, accepted traversal agrees with kernel resolution. Relevant fixture modes are explicitly restored under umask `077`, and `platformDir` resolves a linked temp-root spelling. Basename substitutions and fixed UIDs remain assumptions. |
| + |
| +- **Open-item scope and reports:** the pathname-open gap includes reopens and sidecars, not only initialization. The “crash gap” includes routine process exit. Same-filesystem bind mounts remain undetected when only one alias is inventoried; two listed aliases are rejected by identity. Review-eight, review-nine and review-eleven reports are present. |
| + |
| +Claims the code contradicts: |
| + |
| +- **Removed ranking still described:** `litestream_owners.go:179` describes a strictness ranking and `:187` still includes isolated-instance paths among the directories receiving `namedFile` treatment. The implementation now runs every check and limits that exception to `dataDir/*.db`. `litestream.go:388` still refers to removed `strictestClaim`. |
| + |
| +- **All checks read one inode:** `litestream_owners.go:196` says the facts are one inode’s whichever check read them. The checks perform independent pathname operations and overwrite `facts` without comparing identities. |
| + |
| +- **Only two deliberate omissions:** `litestream_owners.go:591` still says “the two things” although `:618` lists more. Dispositions `:258` says this was corrected. |
| + |
| +- **“Never followed”:** dispositions `:238` retains this description, while `strictTreeDBs` follows links for metadata through `os.Stat`. The updated source comment correctly distinguishes metadata classification from walking or opening contents. |
| + |
| +- **`Accept` names the pair:** dispositions `:252` overstates the fix because cleaning the rendered pair is lossy. The test comment at `litestream_owners_test.go:673` also claims accepted-link protection without exercising `Accept`. |
| + |
| +- **Another owner’s coherent directory is refused:** dispositions `:233` and the test’s introductory comment at `litestream_owners_unix_test.go:332` retain that claim. The corrected case at `:379` asserts acceptance. |
| + |
| +- **No layout lets the tree check accept while the file check refuses:** `litestream_owners_unix_test.go:425` is false. A tenant-owned tree containing a root-owned, world-writable subdirectory and a tenant-owned database passes the tree’s UID/device checks; `tenantFile` refuses the starting directory at `litestream_owners.go:459`. |
| + |
| +- **Close always reads the witness once more:** the unconditional wording remains at `litestream_metered.go:690`, with similar wording at `:476` and `:549`. A stale fleet skips it, including one made stale by the final file look. The amended `Replaces` comment does not correct those other statements. |
| + |
| +- **The opened-store test establishes the right recorder:** `litestream_metered_test.go:920` still says so, while `:980` correctly admits that correspondence is not asserted. |
| + |
| +- **Every new mutation test fails without its fix:** dispositions `:255` needs the non-root qualification for the mixed root/process fixture. Author-reported package and mutation runs are otherwise 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 58c48044c..4322444eb 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -235,7 +235,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review10.md`, `CODEX-FINDINGS-metered-r |
| | 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). | |
| +| 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 walked into or opened (one `Stat` classifies it by where it leads). 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. | | | |
| @@ -249,12 +249,27 @@ Reports: `OPUS-REVIEW-metered-replicator-review11.md`, `CODEX-FINDINGS-metered-r |
| | 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) | |
| +| 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, so a re-pointed link reads as a change; a link whose target cannot be looked at is a problem. [Review twelve: `Accept` cleaned the whole entry as a path, so a target that folds or pops under cleaning passed as the accepted pair; the thirteenth build quotes the target and compares a link entry as written.] 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 | | |
| +| 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` (skipped under root, where the two owners are one and the test would say nothing); `TestAnInstancesDatabaseIsCheckedByItsChain` (an instance directory that is a tenant's link: refused); each fails with its fix removed (mutation runs, not as root) | |
| +| 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 test's introduction said "refused" until the thirteenth build); 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. | | | |
| +| 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, bar the leftovers review twelve found (the old ranking paragraph above the new one, "the two things", `strictestClaim` in `litestream.go`, the witness sentence at `inheritHold` and `Open`), corrected in the thirteenth build. | | | |
| 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. |
| + |
| +## Review twelve (reviewed `c8e5e47b`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-metered-replicator-review12.md`, `CODEX-FINDINGS-metered-replicator-review12.md` (in the tree from the thirteenth build's commit). Both found review eleven's bounded findings closed with tests that fail without them, and both found the same fault in the twelfth build's link entries: `Accept` cleaned the whole `path -> target` string as a path, so a target that folds (`x/../live.sqlite`) or pops (`../../`) under cleaning passed as the accepted pair at the next build, and a target with a line break could forge what the operator is shown. The thirteenth build: |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Opus P3-1, Astra 5 (P2): the link entry was not a safe acceptance key or encoding. | Fixed: a link entry is `path -> "target"` with the target quoted (`strconv.Quote`: separators, line breaks and `..` read as what they are), and `Accept` compares a link entry as written, never cleaned; a plain path is still accepted as cleaned. A regular file whose own name contains ` -> "` and ends in a quote would read as a link entry; that is stated as the residue. Relative targets are rendered as `Readlink` gives them (the classification follows the kernel's resolution, the entry names the spelling). | `litestream_owners.go` `linkEntry`, `isLinkEntry`; `litestream_metered.go` `MeteredFleetStore` | `TestAcceptComparesALinkEntryAsWritten` (the pair accepted builds the store; the link re-pointed to the same file through `x/..` refuses it until the new pair is accepted); `TestStrictTreeNamesALinkToAFile` (a target spelt through `..` is named as written); each fails with the target cleaned, or with link entries cleaned by `Accept` (mutation runs) | |
| +| Opus P3-2: the checks on one file were not required to see one inode. | Fixed: the facts kept are the first check's, and a later check whose `Dev`/`Ino` differ refuses the file ("changed under the look"). No deterministic test: the case is a rename between two checks of one inventory; stated. | `litestream_owners.go` `fleetInventory` | | |
| +| Opus P3-3, Astra 11: the every-claim test could not tell "every check" from "the tree check only", and its comment called the needed layout impossible; the root/platform test is vacuous as root; the sealed-link case is skipped as root. | Fixed: the layout exists (the tree's user's file in another user's directory inside the tree: the tree check accepts, the file check refuses the directory) and is the test's second case; the root/platform test skips as root with a message. The sealed case stays skipped as root (root can look at anything), stated. | tests | `TestEveryClaimOnAFileMustPass`; fails with tree checks alone whenever a tree claim exists (mutation run) | |
| +| Opus P3-4: a `Readlink` error was dropped. | Fixed: ENOENT is churn; any other error is a problem. | `litestream_owners.go` `strictTreeDBs` | no deterministic test (a link that cannot be read by this process while its directory can be listed is not arranged); stated | |
| +| Astra 6 to 9 (P2s), 1 to 4 (P1s), 10: the device open; hibernating aliases and claims on skipped files; required coverage that can disappear (`CONSOLE_DB` naming a missing database; external inventory); the box-wide refusal; litestream's opens by path; ancestry; delayed admission; process exit; the uid binding. | Open, as before; each stated where it applies; the box-wide refusal and the uid binding are decisions before wiring. | | | |
| +| Claims (both): the old ranking paragraph left above the new one; `strictestClaim` named in `litestream.go`; "the two things"; "a link re-pointed later reads as a change" (now true at `Accept` too); "the facts are one inode's" (now checked); the kinds' docs; the test comments ("no layout", "another user's directory refused", "the right recorder", "an accepted link cannot be turned unseen"); the witness-once-more sentences at `inheritHold`, `Open` and `Close`; this file's rows above. | Corrected in place. | | | |
| + |
| +Package tests pass under `/tmp` and `~/.cache` (the author's runs, not as root). The full gate on this tip is next; the thirteenth build has not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review12.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review12.md| new file mode 100644 |
| index 000000000..c3c8de1ec |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review12.md |
| @@ -0,0 +1,136 @@ |
| +# Twelfth review: feat/carloku-metered-replicator-20261001 at `c8e5e47b` |
| + |
| +**Verdict: accept with fixes.** Every bounded finding from review eleven is closed, and each new test fails without its fix, with one exception: the root/platform-uid test is vacuous when run as root. The twelfth build adds one new weakness of its own. `Accept` compares `filepath.Clean`ed strings, so the new `link -> target` pairing is not exact: a re-pointed link can still be accepted at the rebuild. The changes to the metered fleet are comments only, and the registry is unchanged, so B, C, D, I, M and the latch questions stand as review eleven found them. |
| + |
| +I read the source with `git show c8e5e47b:` and ran nothing. |
| + |
| +## Review eleven, finding by finding |
| + |
| +| Finding | At `c8e5e47b` | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P2-1 / Astra 9:** link skip noise, re-pointed links, `Stat` errors | **Closed for the targets and the `Stat` errors** (`litestream_owners.go:652-660`). The box-wide refusal is stated as a decision to make before wiring (`:612-616`), so it is still open and still blocks wiring. The pairing is weakened by `Clean` in `Accept` (P3-1 below). | **No.** The re-point check and the "sealed" check each fail without their case. The sealed case runs only when not root (`litestream_owners_test.go:687`). | |
| +| **Opus P3-1 / Astra 5:** ranked claim kinds | **Closed** (`claimChecks`, `:259-289`; loop `:197-210`). | **No.** `strictestClaim` and "first check only" both accept. But "tree check only, when a tree claim exists" passes the test (P3-3). | |
| +| **Opus P3-2:** root and the platform uid; `activatorFile`'s premise | **Closed** (`:539`, `:240-243`). Whether a tenant-uid file in a platform directory is billable is stated as not decided. | Instance test: **no** (under `namedFile` the tenant's link is followed). Root/platform test: **no** as non-root; **yes** as root, where uid 0 equals uid 0 without the fix. | |
| +| **Opus P3-3 / Astra 7:** a unit tree over a platform file; aliases; skipped files | **Closed for the tree** (`routeClaiming`, `:222-233`, the same predicate as `:170`). Aliases and hibernating claims on skipped files are open, as stated. | **No.** The console-directory case fails with exact spellings alone. | |
| +| **Astra 6:** a hibernating unit under a hibernating exec claim | **Closed** (`litestream.go:713`). The `hibernating` map comes only from non-unit routes (`:381`), so a unit never counts against itself. | **No.** | |
| +| **Opus P3-4 / Astra 11:** tests | **Closed for the B fixture**, which is now a coherent B tree and asserts the acceptance that Astra 10 predicts; the every-claim test is added. The other items are open as listed. | n/a | |
| +| **Astra 1-4, 8, 10:** opens by path, ancestry, delayed admission, crash gap, device open, uid binding | **Open**, unchanged and stated. | n/a | |
| +| **Claims (both)** | Mostly corrected. Three stale passages remain (see Claims). | n/a | |
| + |
| +## Findings |
| + |
| +### P3-1. `Accept` cleans the `link -> target` string, so the pairing it is meant to pin is not exact (T) |
| + |
| +**Where:** `litestream_metered.go:853-861` cleans both the `Accept` entries and the skipped entries. The entries are built at `litestream_owners.go:656-660`. |
| + |
| +**What goes wrong:** `filepath.Clean` treats the whole `p -> dest` string as a path. |
| + |
| +- **Folding inside the target:** `.. ` components in the target fold. |
| + - Accepted: `/var/lib/carlos-app/u/current -> /var/lib/carlos-app/u/live.sqlite`. |
| + - The tenant re-points `current` at `/tmp/x/../../var/lib/carlos-app/u/live.sqlite`, where `/tmp/x` is the tenant's own link (PrivateTmp, outside the walked tree). |
| + - The running fleet sees drift, because `render()` compares raw strings. |
| + - The rebuild's `Accept` check sees the same cleaned string and passes. The kernel resolves the new target through the tenant's link, to a different file. |
| +- **Folding across the separator:** a `..` right after ` -> ` pops the link's own name. |
| + - `/d/zz -> ../../old.sqlite` cleans to `/d/old.sqlite`. |
| + - It is therefore accepted by an existing plain skip entry for `/d/old.sqlite`. |
| +- **No cleaning needed:** a regular database file named `current -> live.sqlite` renders exactly as the accepted link entry `current -> live.sqlite`. Each accepts the other. |
| +- **Newlines:** a target containing `\n` makes the entry, the error text (joined by `, `) and `render()` (joined by `\n`) ambiguous. It fails closed at `Accept`, but it can forge what an operator is shown. |
| + |
| +**Bound:** a skipped file is neither replicated nor billed, so the cost is a tenant's own durability, not attribution. The claim at `:610-611` ("a link re-pointed later reads as a change") does not hold across a rebuild. |
| + |
| +**Fix:** quote both halves (`strconv.Quote`), or carry skipped entries as `{path, target}`, and compare `Accept` literally with no `Clean` for link entries. |
| + |
| +### P3-2. The checks on one file are not required to see one inode, so the facts kept are the last check's (S) |
| + |
| +**Where:** `litestream_owners.go:197-207`. The comment at `:195-196` says "the facts are one inode's, whichever check read them". |
| + |
| +**Scenario:** |
| +1. File check accepts inode X. |
| +2. The tenant renames a directory within its own tree. |
| +3. The tree check reads inode Y and accepts it. |
| +4. `facts = Y`, and later looks compare against Y, an inode the file check never passed. Its file-only restrictions (the owner of the file's directory) were never applied to Y. |
| + |
| +The order is load-bearing in this one way. It needs a race within one inventory, so P3. |
| + |
| +**Fix:** keep the first check's facts and refuse the file when any later check's `Dev`/`Ino` differs. |
| + |
| +### P3-3. The every-claim test cannot tell "every check" from "the tree check only", and its comment says that case cannot be built (H, S) |
| + |
| +**Where:** `litestream_owners_unix_test.go:422-428`. |
| + |
| +- **The layout the comment calls impossible can be built** (`withOwners` keys by base name): |
| + - `/srv/A` owned by T; |
| + - `/srv/A/o` owned by O; |
| + - `/srv/A/o/main.db` owned by T. |
| +- **What each check does with it:** |
| + - The tree check from `/srv/A` asks nothing of intermediate owners and accepts. |
| + - The file check refuses at `litestream_owners.go:462`. |
| +- **The mutation that survives:** running only tree checks whenever a tree claim exists passes every test. |
| +- **Root-run vacuity:** |
| + - `TestRootAndThePlatformUserAreOneOwner` passes without its fix under root. |
| + - The sealed-link case is skipped under root. |
| + - Whether CI runs as root decides whether either is exercised. |
| + |
| +**Fix:** add the O-directory case, expecting a refusal with both claims present. Make the root test use a fake non-zero platform uid, or skip it with a message under root. |
| + |
| +### P3-4. A `Readlink` error is dropped (L) |
| + |
| +**Where:** `litestream_owners.go:653`. |
| + |
| +**Scenario:** the entry is a link at `Lstat`, then is replaced by a regular file before `Readlink`. `Stat` sees a regular file, so the skipped entry is `p -> ` with an empty target. It looks like a link with no target and passes over the fact that the entry changed. |
| + |
| +**Fix:** a `Readlink` error other than ENOENT is a problem. On ENOENT, drop the entry as churn. |
| + |
| +## The lettered questions |
| + |
| +- **A, J, U (the chain):** |
| + - **Own-uid tenant:** a link or another user's directory in the chain is still refused at `:452-463`, `:475`, `:480`, `:489` and `:492`. The top-of-chain link check is reached on every accepting path. |
| + - **What is above the chain:** only the directory directly above the top is checked. In the every-claim fixture, the root-owned `bridge` sits inside T's `/srv/A`, so T can replace it after the look (Astra 2, open). |
| + - **`platformUID` at `:539`:** it adds acceptance only when both the file and the top of the chain are platform uids. With the replicator running as root it adds nothing. |
| + - A tenant cannot pass off a root-owned file in its own directory: it is refused at `:462`, or at `platformOwned` when the file sits under a root subdirectory. |
| + - The reverse (the tenant's file in a platform directory) is refused, as stated. |
| + - **Platform-uid tenants and root-run units:** the climb stops at once, and such a file is accepted wherever its directory is the platform's (open, approval H). |
| + - **Isolated instances:** `/data/instances/<iid>` is `0700 carlos:carlos` (design §3.1). As `tenantFile`, it is accepted whether the replicator runs as root (the chain climbs to `/data/instances`, and `/data` is the platform's) or as `carlos`. It is not refused wrongly. |
| + - **DynamicUser state directories** named by an exec route resolve through the root-made link and are accepted. |
| + - **A `DBPath` that is a link** to another tenant's file is refused at `:416`. |
| +- **B, I:** unchanged. |
| + - The three stale checks (`litestream_metered.go:212`, `:224`, `:238`) are complete. |
| + - The retries are bounded at three. Only a witnessed change or a persistent error counts as a change. |
| + - `closeOnce` holds, and `closed` is set last (`:770`). |
| + - The delayed admission across midnight (Astra 3) is open. |
| +- **C:** unchanged and correct. |
| +- **D, M, the latch:** |
| + - The order is unchanged: store stopped, watch stopped, look, witness, latch, closed (`:723-770`). |
| + - The inherited window is applied once. |
| + - A day can still go unheld on some recorder in two cases, both stated as open: |
| + - a swap made and restored between the last clean look and Close's look, including during the store's final sync; |
| + - Astra 3's sequence: admitted at 23:58, the change, the successor closed unopened, recorded at 00:02, so October 3 is held by nobody. |
| +- **E (lineage):** the key is account plus path (`:922-924`). A→B→A at one path resumes A's lineage, which the code comment states. A path renamed between owners gets a new key under the new account. |
| +- **F, N, L:** nothing is newly silent. |
| + - Names are matched case-insensitively, and every non-empty unrecognised regular file is header-read. |
| + - A directory that cannot be read is a problem, and the new `Stat`-error case is a problem too. |
| + - A unit route with no `DBPath`, or a relative one, is reported (`litestream.go:367-377`), and empty or vanished claimed files are skipped (`:521-536`). |
| + - Explicit claims accept nothing the owner checks refuse, and the external config is unchanged. |
| +- **Q (still silent):** a hibernating claim spelled through an alias, and a hibernating claim on a file the walk only skipped. Both are stated as open. |
| + - Two live routes, or a route and the platform, claiming one inode through different spellings are caught by the identity check (`:129-140`). |
| +- **R (link noise):** still the wiring-blocking decision. |
| + - Directory links (`venv/lib64`, pnpm's `node_modules`) are problems with no `Accept` route at all. |
| + - With targets now rendered, an OS upgrade that changes `venv/bin/python`'s target is also drift and a fresh accept. |
| + - A link that reaches a file through a directory link is classified by its final target, which is correct. |
| +- **G:** stated correctly. None of the open items is smaller than stated. |
| +- **K:** unchanged. `sidecarOf` is gone, and platforms without `O_NOFOLLOW` refuse the header read. |
| +- **O:** the review-eight to review-eleven reports are in the tree. |
| + |
| +## Claims the code contradicts |
| + |
| +- **`litestream_owners.go:179-190`:** "follows the strictest claim on it" and "(dataDir/*.db, dataDir/instances/*/*.db)". The old paragraph was left above the new one and is contradicted by `:191-210` and `activatorFile`. |
| +- **`litestream.go:385-390`:** "checks it as a tenant's named file (strictestClaim …)". `strictestClaim` is gone, and the check is a `tenantFile`. |
| +- **`litestream.go:705`:** `liveClaim` is "a route that is not hibernating". It now includes hibernating unit routes. |
| +- **`litestream_owners.go:591-593`:** "the two things it passes over on purpose". The list at `:618-623` has four. |
| +- **`litestream_owners.go:610-611`, and the dispositions row for Opus P2-1:** "a link re-pointed later reads as a change". True of `render()`; not true of `Accept` at the rebuild (P3-1). |
| +- **`litestream_owners.go:195-196`:** "the facts are one inode's". Not checked (P3-2). |
| +- **`litestream_owners.go:326-336`:** `namedFile` names "the activator's own directories" (there is now one), and `tenantFile` is "a unit route names the file itself" (it is also any route naming a file outside `dataDir/*.db`). |
| +- **`litestream_owners_unix_test.go:425`:** "No layout a test can build has the tree accept and the file check refuse" (P3-3). |
| +- **Dispositions, "Package tests pass":** these are the author's runs, neither confirmed nor contradicted here. |
| + |
| +**Verdict: accept with fixes.** Fix P3-1 to P3-4 and the stale comments. Astra 1-4 and 8, approval H's uid binding, and the box-wide refusal decision still block wiring. |
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go| index 78f7d56da..7a8bb1175 100644 |
| --- a/internal/ops/litestream.go |
| +++ b/internal/ops/litestream.go |
| @@ -384,8 +384,8 @@ 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, since a route named it, and checks it as a tenant's named |
| - // file (strictestClaim: nothing is known of who can write in its |
| + // it, since a route named it, and checks it as a tenant's file |
| + // (claimChecks: 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 != "" { |
| @@ -702,8 +702,10 @@ func sortedKeys(m map[string]bool) []string { |
| 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 "". |
| +// liveClaim is the host of a route whose database is this fleet's (any |
| +// unit route, hibernating or not; an exec route that is not hibernating) |
| +// and that 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 { |
| // A unit route's database is this fleet's whether or not the unit |
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go| index 5f11b2d31..9582e63d5 100644 |
| --- a/internal/ops/litestream_metered.go |
| +++ b/internal/ops/litestream_metered.go |
| @@ -473,8 +473,9 @@ func (f *MeteredFleet) latch() (from, at time.Time, stale, first bool) { |
| // inheritHold reads, once, the window the fleet this one replaces held, |
| // tells this fleet's recorders, and keeps it to pass on. It is called at |
| // Open and at Close, and takes effect only once every fleet in the chain |
| -// has closed (closing asks the witness one last time, so whatever the old |
| -// fleet had to say about its last days it has said by then): a fleet |
| +// has closed (closing looks at the files and asks the witness one last |
| +// time each, unless the fleet is stale already, so whatever the old fleet |
| +// had to say about its last days it has said by then): a fleet |
| // closed before its predecessor does not latch, and answers for its |
| // predecessor through staleWindow until a later fleet reads it. |
| func (f *MeteredFleet) inheritHold() { |
| @@ -546,8 +547,9 @@ func (f *MeteredFleet) Open(ctx context.Context) error { |
| if !f.chainClosed() { |
| return errors.New("ops: the replicator this one replaces has not been closed") |
| } |
| - // The fleet this one replaces has closed, and closing asks the witness |
| - // one last time, so whatever it had to say about its last days it has |
| + // The fleet this one replaces has closed, and closing looks at the |
| + // files and asks the witness one last time each (unless it was stale |
| + // already), so whatever it had to say about its last days it has |
| // said by now. That is told to this fleet's recorders here, once: a |
| // database that changed hands has a new owner, whose recorder the old |
| // fleet never had. Read at build it would be missed whenever the new |
| @@ -688,8 +690,9 @@ func (f *MeteredFleet) Databases() int { |
| } |
| // 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 |
| +// asks the witness one last time each (a fleet already stale skips both, |
| +// a fleet made stale by the look skips the witness), 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 |
| @@ -850,13 +853,25 @@ func MeteredFleetStore(o MeteredFleetOptions) (fleet *MeteredFleet, err error) { |
| if len(inv.Problems) > 0 { |
| return nil, fmt.Errorf("%w: places a database could be that cannot be vouched for: %s", ErrUnmeterable, strings.Join(inv.Problems, "; ")) |
| } |
| + // A plain path is accepted as cleaned; a link entry (path -> "target") |
| + // as written, so the pair the operator accepted is the pair that is |
| + // there, and a target that folds or pops under cleaning cannot pass |
| + // as it. |
| accepted := map[string]bool{} |
| for _, p := range o.Accept { |
| - accepted[filepath.Clean(p)] = true |
| + if isLinkEntry(p) { |
| + accepted[p] = true |
| + } else { |
| + accepted[filepath.Clean(p)] = true |
| + } |
| } |
| var unseen []string |
| for _, p := range inv.Skipped { |
| - if !accepted[filepath.Clean(p)] { |
| + key := p |
| + if !isLinkEntry(p) { |
| + key = filepath.Clean(p) |
| + } |
| + if !accepted[key] { |
| unseen = append(unseen, p) |
| } |
| } |
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go| index fc6b05ccf..0484a9a7b 100644 |
| --- a/internal/ops/litestream_metered_test.go |
| +++ b/internal/ops/litestream_metered_test.go |
| @@ -918,7 +918,9 @@ func TestMeteredFleetLooksAtTheFilesAgain(t *testing.T) { |
| // through the fleet's clients to a server that counts. |
| // |
| // It shows what the pieces above cannot: that the requests litestream makes |
| -// of its own accord are counted on the right recorder; that the replicator |
| +// of its own accord are counted (on some recorder of the fleet's; which |
| +// database's is asserted by the lease package's own test, not here); that |
| +// the replicator |
| // writing its own bookkeeping into the registry it replicates does not |
| // make the fleet stale; that the shadow state stays the platform's once |
| // litestream has initialised it; that after a change of ownership nothing |
| @@ -1850,3 +1852,45 @@ func TestCloseHoldsWhenItsLastLookCannotBeTaken(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// Accept compares a link entry as written: the pair the operator accepted |
| +// is the pair that must be there, and a target re-pointed to spell the |
| +// same file through a ".." (which cleaning would fold away) is not it. |
| +// (Review twelve, Opus P3-1.) |
| +func TestAcceptComparesALinkEntryAsWritten(t *testing.T) { |
| + newFakeBucket(t) |
| + dir := platformDir(t) |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + state := filepath.Join(platformDir(t), "u") |
| + mkdirs(t, state) |
| + db := writeFile(t, filepath.Join(state, "main.db")) |
| + live := writeFile(t, filepath.Join(state, "live.sqlite")) |
| + if err := os.Symlink(live, filepath.Join(state, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + upsert(t, reg, registry.Route{Host: "u.example.com", Account: "bac", App: "one", DBPath: state, Backing: registry.BackingUnit}) |
| + o := fleetOptions(reg, dir) |
| + if _, err := MeteredFleetStore(o); !errors.Is(err, ErrUnmeterable) { |
| + t.Fatalf("a link to a file not accepted: %v; want the store refused", err) |
| + } |
| + entry := linkEntry(filepath.Join(state, "current"), live) |
| + o.Accept = []string{entry, live} // live.sqlite is itself a skipped name |
| + fleet, err := MeteredFleetStore(o) |
| + if err != nil { |
| + t.Fatalf("the accepted pair: %v", err) |
| + } |
| + fleet.Close() |
| + // Re-pointed to the same file through a "..": another pair. |
| + if err := os.Remove(filepath.Join(state, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + mkdirs(t, filepath.Join(state, "x")) |
| + // Spelt as a string: filepath.Join would clean the ".." away. |
| + if err := os.Symlink(state+"/x/../live.sqlite", filepath.Join(state, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := MeteredFleetStore(o); !errors.Is(err, ErrUnmeterable) { |
| + t.Fatalf("the link re-pointed through a ..: %v; want the store refused until the new pair is accepted", err) |
| + } |
| + _ = db |
| +} |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index 361dd5774..00c2e7ba4 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -8,6 +8,7 @@ import ( |
| "os" |
| "path/filepath" |
| "sort" |
| + "strconv" |
| "strings" |
| "github.com/carlosframework/platform/internal/registry" |
| @@ -176,34 +177,30 @@ func fleetInventory(routes []registry.Route, registryPath, dataDir string) (Flee |
| unowned(p, "no single account claims it") |
| continue |
| } |
| - // 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. |
| // 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. |
| + // and kind the claims imply (claimChecks), and one refusal refuses |
| + // it. The checks must all have read one inode: the facts kept are |
| + // the first check's, and a later check that read another file (a |
| + // rename under the look) refuses, since that file passed only the |
| + // later checks. |
| var facts fileFacts |
| refused := false |
| - for _, c := range claimChecks(claims, p, dataDir) { |
| + for i, c := range claimChecks(claims, p, dataDir) { |
| f, why := checkOwnedFile(c.root, p, c.kind) |
| + if why == "" && i > 0 && (f.Dev != facts.Dev || f.Ino != facts.Ino) { |
| + why = "it changed under the look (its claims' checks did not all see one file)" |
| + } |
| if why != "" { |
| unowned(p, why) |
| refused = true |
| break |
| } |
| - facts = f |
| + if i == 0 { |
| + facts = f |
| + } |
| } |
| if refused { |
| continue |
| @@ -323,16 +320,17 @@ 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 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: an exec route names a file in the activator's own flat |
| + // directory, dataDir/*.db (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 |
| - // be the tenant's and contain no link, and the directory above that |
| - // must be the platform's: the tenant can write anywhere in its own |
| - // chain, and a link anywhere in it leads wherever the tenant likes. |
| + // tenantFile: a unit route names the file itself, or any route names |
| + // a file outside the activator's directory. The file's whole chain of |
| + // directories, up to where its owner's ownership stops, must be that |
| + // owner's and contain no link, and the directory above that must be |
| + // the platform's: the owner can write anywhere in its own chain, and |
| + // a link anywhere in it leads wherever the owner likes. |
| tenantFile |
| ) |
| @@ -588,8 +586,7 @@ 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 (the two things it passes over on purpose, an |
| -// entry that vanishes under the look and an empty file, are listed at |
| +// places it did not look (what it passes over on purpose is listed at |
| // the end): |
| // |
| // - a database it would not replicate is reported in skipped: one in a |
| @@ -609,7 +606,9 @@ const strictTreeMaxDepth = 32 |
| // 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 |
| +// change to the running fleet and to the Accept list at the next |
| +// build alike (the target is quoted, and a link entry is compared |
| +// as written). 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 |
| @@ -647,17 +646,26 @@ func strictTreeDBs(root string) (dbs, skipped, problems []string) { |
| 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. |
| + // A link entry is named with its target, quoted, so a |
| + // link re-pointed after it was accepted reads as a change |
| + // and no target can forge another entry or a line break. |
| + dest, rerr := os.Readlink(p) |
| + if errors.Is(rerr, fs.ErrNotExist) { |
| + continue // gone under the look: churn |
| + } |
| + if rerr != nil { |
| + problems = append(problems, p+": a link that cannot be read") |
| + continue |
| + } |
| + entry := linkEntry(p, dest) |
| target, terr := os.Stat(p) |
| - dest, _ := os.Readlink(p) |
| switch { |
| case sqliteName(name) || (terr == nil && target.IsDir()): |
| - problems = append(problems, p+" -> "+dest+": a link that may lead to a database") |
| + problems = append(problems, entry+": 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") |
| + problems = append(problems, entry+": a link whose target cannot be looked at") |
| case terr == nil && target.Mode().IsRegular(): |
| - skipped = append(skipped, p+" -> "+dest) |
| + skipped = append(skipped, entry) |
| } |
| case fi.IsDir(): |
| if depth+1 > strictTreeMaxDepth { |
| @@ -833,3 +841,18 @@ func sqliteHeader(path string, looked os.FileInfo) (bool, error) { |
| func platformUID(uid uint32) bool { |
| return uid == 0 || int(uid) == os.Geteuid() |
| } |
| + |
| +// linkEntry is how a link is named in an inventory: its path, then its |
| +// target as Readlink gave it, quoted. The quoting keeps a target with a |
| +// separator, a line break or a ".." from reading as another path, and |
| +// Accept compares such an entry as written (isLinkEntry), never cleaned. |
| +func linkEntry(path, target string) string { |
| + return path + " -> " + strconv.Quote(target) |
| +} |
| + |
| +// isLinkEntry reports whether an inventory entry names a link with its |
| +// target (linkEntry). |
| +func isLinkEntry(entry string) bool { |
| + i := strings.Index(entry, " -> \"") |
| + return i > 0 && strings.HasSuffix(entry, "\"") |
| +} |
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go| index a180c239f..ae0ddf05f 100644 |
| --- a/internal/ops/litestream_owners_test.go |
| +++ b/internal/ops/litestream_owners_test.go |
| @@ -667,11 +667,12 @@ 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")+" -> "+target) || containsPrefix(skipped, filepath.Join(root, "dangling")) { |
| + if len(dbs) != 0 || len(problems) != 0 || !contains(skipped, linkEntry(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. |
| + // Re-pointed, the entry reads differently, to the running fleet's |
| + // render and to the Accept list at the next build alike |
| + // (TestAcceptComparesALinkEntryAsWritten). |
| if err := os.Remove(filepath.Join(root, "current")); err != nil { |
| t.Fatal(err) |
| } |
| @@ -680,9 +681,24 @@ func TestStrictTreeNamesALinkToAFile(t *testing.T) { |
| t.Fatal(err) |
| } |
| _, skipped, _ = strictTreeDBs(root) |
| - if !contains(skipped, filepath.Join(root, "current")+" -> "+other) || contains(skipped, filepath.Join(root, "current")+" -> "+target) { |
| + if !contains(skipped, linkEntry(filepath.Join(root, "current"), other)) || contains(skipped, linkEntry(filepath.Join(root, "current"), target)) { |
| t.Fatalf("re-pointed: skipped %v; want the new target named", skipped) |
| } |
| + // A target with a line break or a ".." is quoted, not folded: the |
| + // entry names exactly the target as written. |
| + if err := os.Remove(filepath.Join(root, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + mkdirs(t, filepath.Join(filepath.Dir(other), "x")) |
| + // Spelt as a string: filepath.Join would clean the ".." away. |
| + odd := filepath.Dir(other) + "/x/../" + filepath.Base(other) |
| + if err := os.Symlink(odd, filepath.Join(root, "current")); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, skipped, _ = strictTreeDBs(root) |
| + if !contains(skipped, linkEntry(filepath.Join(root, "current"), odd)) || contains(skipped, linkEntry(filepath.Join(root, "current"), other)) { |
| + t.Fatalf("a target spelt through ..: skipped %v; want it named as written", skipped) |
| + } |
| // A link whose target cannot be looked at is a problem, not nothing. |
| if os.Geteuid() != 0 { |
| locked := filepath.Join(platformDir(t), "locked") |
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go| index 32388ad7b..549182703 100644 |
| --- a/internal/ops/litestream_owners_unix_test.go |
| +++ b/internal/ops/litestream_owners_unix_test.go |
| @@ -329,10 +329,12 @@ func TestAPlatformLinksRelativeTargetIsWalkedNotCleaned(t *testing.T) { |
| } |
| } |
| -// 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 |
| +// An exec route's database outside the activator's directory is checked |
| +// as a tenant's file: one whose directory is a link the tenant made is |
| +// refused; one in the tenant's own chain beneath the platform's directory |
| +// is owned; and another user's file in that user's own coherent chain is |
| +// accepted as the route's, since nothing binds a route to a uid (open, |
| +// approval H). (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) { |
| @@ -419,12 +421,33 @@ func TestEveryClaimOnAFileMustPass(t *testing.T) { |
| 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".) |
| + // The other way round (review twelve, Opus P3-3): the tree check asks |
| + // nothing of a directory's owner inside the tree, so the tree's user's |
| + // file in another user's directory inside the tree passes it; the |
| + // file check refuses that directory. With both claims the file is |
| + // refused, so "tree checks only" is told from "every check". |
| + top2 := platformDir(t) |
| + mkdirs(t, filepath.Join(top2, "srv", "C", "o")) |
| + inTree := writeFile(t, filepath.Join(top2, "srv", "C", "o", "tree.db")) |
| + withOwners(t, map[string]uint32{"srv": 0, "C": tenant, "o": other, "tree.db": tenant}) |
| + dir2 := platformDir(t) |
| + reg2 := openRegistry(t, filepath.Join(dir2, "registry.db")) |
| + upsert(t, reg2, registry.Route{Host: "c.example.com", Account: "bac", App: "one", DBPath: filepath.Join(top2, "srv", "C"), Backing: registry.BackingUnit}) |
| + inv, err = FleetDBOwners(reg2, dir2) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[inTree]; !ok { |
| + t.Fatalf("fixture: the tree check alone refused: %v", inv.Unowned) |
| + } |
| + upsert(t, reg2, registry.Route{Host: "f.example.com", Account: "bac", App: "one", DBPath: inTree}) |
| + inv, err = FleetDBOwners(reg2, dir2) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := ownedByPath(inv)[inTree]; ok || !containsPrefix(inv.Unowned, inTree) { |
| + t.Fatalf("two claims, the file check refusing: owned %+v, unowned %v; want refused", inv.Owned, inv.Unowned) |
| + } |
| } |
| // An isolated instance's database (dataDir/instances/<id>/<host>.db) is |
| @@ -457,6 +480,9 @@ func TestAnInstancesDatabaseIsCheckedByItsChain(t *testing.T) { |
| // directory is the platform's, not a tenant's in someone else's directory. |
| // (Review eleven, Opus P3-2.) |
| func TestRootAndThePlatformUserAreOneOwner(t *testing.T) { |
| + if os.Geteuid() == 0 { |
| + t.Skip("as root the platform's user is root: the two owners this test tells apart are one") |
| + } |
| top := platformDir(t) |
| mkdirs(t, filepath.Join(top, "srv", "ops")) |
| file := writeFile(t, filepath.Join(top, "srv", "ops", "archive.sqlite")) |