amadan
Review twenty answered: comments scoped to the tenant claims, a depth fixture, and the two refusals that are decisions listed as decisions
Opus accept with fixes; Astra reject on the deep items, unchanged. No behaviour changes: the comments say which claims the platform-directory refusal covers and what the two tenant claims still differ on by design; a fixture tells the immediate directory's owner from the first component's; the dispositions list, as decisions and not as changes, whether a tenant's file in a platform directory is billable and whether a tree claim should apply the file claim's chain rules, since each changes which boxes refuse their whole build. Ops passes.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
7 files changed,
+358
−18
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review20.md+205 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+24 −2docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review20.md+90 −0internal/ops/litestream_metered_test.go+3 −1internal/ops/litestream_owners.go+15 −7internal/ops/litestream_owners_test.go+4 −3internal/ops/litestream_owners_unix_test.go+17 −5
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review20.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review20.md| new file mode 100644 |
| index 000000000..ed81b96d0 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review20.md |
| @@ -0,0 +1,205 @@ |
| +| Review-sixteen finding | Closed at `c9ec11eb`? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 — fleet versus tenant mount namespace** | **Closed as a wording request.** Namespace divergence remains explicitly open. | Wording only; no behavioral test. | |
| +| **Opus T-1 — aliased tree root** | **Closed.** The fixture includes `state -> private/state`, with `cur -> ../d`. | **No** with lexical joining; **yes** with uncleaned concatenation, which correctly handles this particular case. | |
| +| **Opus T-1 — rendering coverage** | **Closed for the reported mutations.** Owner fields, `Platform`, individual lists and section boundaries have assertions. | **No** with the raw owner triple, omitted `Platform`, raw list entries or removed section separators. | |
| +| **Opus claims** | Namespace wording, historical qualifications and the distinguishing Accept fixture description are corrected. Minor wording issues remain below. | Prose corrections have no behavioral test. | |
| +| **Astra 1 — Litestream pathname opens** | **Open.** | Existing tests permit the defect; none controls substitution at Litestream’s actual open. | |
| +| **Astra 2 — replaceable ancestry** | **Open.** | Existing endpoint checks do not establish protected ancestry. | |
| +| **Astra 3 — delayed admission escapes holds** | **Open.** | No test suspends an admitted attempt beyond the margin across midnight. | |
| +| **Astra 4 — process exit loses uncertainty** | **Open.** | No durable handover or recovery test. | |
| +| **Astra 5 — concatenation exceeds `PATH_MAX`** | **Closed on supported Unix platforms with stable directories.** The fixture now addresses the reported short-root, long-root and XFS-target cases. | **No** when restoring concatenation, provided fixture construction succeeds. | |
| +| **Astra 6 — header-open device effects** | **Open.** The separate directory-open FIFO/device defect is closed. | Header tests permit ancestor substitution. The directory FIFO test fails without `O_DIRECTORY`. | |
| +| **Astra 7 — hibernating aliases and skipped-file claims** | **Open.** | No distinguishing identity-based claimant reconciliation test. | |
| +| **Astra 8 — required coverage disappears** | **Open.** | Missing configured console databases and external-inventory reconciliation remain uncovered. | |
| +| **Astra 9 — whole-fleet refusal and holds** | **Open and acknowledged.** | Existing tests expect this broad behavior; they do not establish tenant isolation. | |
| +| **Astra 10 — route identity versus filesystem authority** | **Open.** | The coherent foreign-tree fixture explicitly expects acceptance. | |
| +| **Astra 11 — weak tests** | **Partly closed.** Rendering, directory-open refusal and this build’s positive acceptance assertion distinguish their reported mutations. Shutdown, watch, interleaving and timing gaps remain. | **No** for the repaired mutations; **yes** for the surviving counterfactuals below. | |
| +| **Astra claims** | Rendering, witness wording, inventory diagnostics and the direct-client attribution qualification are corrected. | Recorded execution results remain author reports, not independently verified here. | |
| + |
| +All locations below are at **`c9ec11eb`**. I reviewed against `72b4f59a`, including the twentieth-build diff from `53e3dcd8`. Source was read through `git show`; nothing was modified, and no tests or network commands were run. Test counterfactuals are deductions from source. |
| + |
| +The requested `*review20.md` reports are absent at this commit. The dispositions reference `*review19.md`, which are present. Both reviewers’ review-eight, review-nine and review-seventeen reports are also present. |
| + |
| +The twentieth build’s bounded changes are correct by inspection: |
| + |
| +| Change | Assessment | |
| +|---|---| |
| +| Tree-claim refusal, `internal/ops/litestream_owners.go:538`, `:555`, `:580` | `dirUID` is the immediate directory’s owner for a file directly under the root, one level down, or deeper. Removing the refusal fails the added assertion at `litestream_owners_unix_test.go:541`. | |
| +| Fixture budget, `internal/ops/litestream_owners_test.go:769` | The unreachable guard is replaced with a check before deep-directory/file creation. Ordinary runs would still pass without this diagnostic check; the separate length and classification assertions distinguish concatenation. | |
| +| Valid platform-file assertion, `internal/ops/litestream_owners_unix_test.go:546` | Now requires acceptance. An unrelated refusal can no longer satisfy it. | |
| +| No-recorder diagnostic, `internal/ops/litestream_metered.go:938` | The owned pathname is now quoted. The existing ordinary-filename assertion at `litestream_metered_test.go:283` would still pass without quoting. | |
| +| Disposition separation, `DISPOSITIONS-metered-replicator.md:372`, `:381` | Accurately separates these changes from the deferred mechanisms. The new refusal enforces the stated conservative treatment of an undecided layout; it does not resolve that policy decision. | |
| + |
| +**1. P1 — Litestream’s actual opens remain outside the ownership checks.** |
| + |
| +**Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:382`; `third_party/litestream/db.go:1004`, `:1014`, `:1032`, `:1866`. |
| + |
| +A tenant passes inventory, substitutes another tenant’s database during initialization, then restores its original pathname before the next look. Litestream can retain substituted handles while subsequent inventories—including Close’s final look—see the original inode. Initialization enables WAL and writes bookkeeping tables; later WAL opens independently follow mutable paths. |
| + |
| +This is larger than a swap during the first open: it includes initialization effects, retained handles and subsequent database/sidecar opens. |
| + |
| +**Smallest acceptable fix:** constrain the actual SQLite, database and sidecar opens through verified, pinned directory authority, including subsequent opens. A check after an unrestricted open cannot undo its effects. |
| + |
| +**2. P1 — Protected directories can still have tenant-replaceable ancestors.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:414`, `:520`, `:604`, `:840`, `:868`; `internal/ops/litestream_metered.go:988`. |
| + |
| +T owns `/srv/T`; root owns `/srv/T/bridge`; A owns `bridge/A` and its database. A platform-made link to `bridge/A` passes the immediate-parent checks, although T can rename and replace `bridge` through `/srv/T`. |
| + |
| +Equivalent layouts work above `dataDir`, shadow state and the directory containing an approved platform link. A link whose target’s **immediate parent** is tenant-owned is refused; higher ancestry remains unchecked. |
| + |
| +**Smallest acceptable fix:** establish and pin trusted ancestry for claim roots, approved platform links and shadow state. Endpoint ownership and modes are insufficient. |
| + |
| +**3. P1 — An admitted attempt delayed beyond the margin escapes the inherited hold.** |
| + |
| +**Locations:** `internal/ops/litestream_metered.go:240`, `:325`, `:789`; `internal/lease/requestgate.go:58`. |
| + |
| +A concrete UTC sequence: |
| + |
| +1. A admits an attempt on October 2 at **23:58:00**, then pauses before recording. |
| +2. Ownership changes at **23:58:10**. A detects it and holds through **23:59:10**. |
| +3. B is built for the new owner and closes unopened while A remains live. |
| +4. A’s admitted attempt resumes, records and completes at **00:02 on October 3**. |
| +5. A closes. Already stale, it skips the final checks and does not extend its window. |
| +6. C replaces B. B and C inherit A’s window correctly, but **October 3 is absent from their ownership holds**. |
| + |
| +`goStaleFrom` correctly chooses the earlier lower bound. Unopened-fleet inheritance preserves the supplied window; neither supplies its missing upper bound. |
| + |
| +**Smallest acceptable fix:** track admitted attempts through recording/completion and include their doubtful dates before finalizing handover. Increasing a fixed margin is insufficient. |
| + |
| +**4. P1 — Ownership uncertainty disappears across process exit.** |
| + |
| +**Locations:** `internal/ops/litestream_metered.go:346`, `:379`, `:834`. |
| + |
| +An attempt passes admission, ownership changes, and the attempt completes before invalidation. The process exits. Its successor reads the new owner without a predecessor or durable uncertainty covering the completed attempt. |
| + |
| +This includes routine restarts, not only crashes, and inheritance awaiting delivery to successor recorders. An unfinished ledger row covers a different case. |
| + |
| +**Smallest acceptable fix:** persist ownership/admission uncertainty before affected requests escape, and recover it before establishing successor coverage. |
| + |
| +**5. P2 — Directory-only opening still permits classification against a substituted directory.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:671`, `:700`, `:719`, `:726`; `internal/ops/linktarget_unix.go:46`. |
| + |
| +The walker lists `D/cache` and captures `current`’s relative target. Before `statTarget` opens `D/cache`, the tenant replaces it with another directory—or a link to another directory—where that target is absent. `Fstatat` returns `ENOENT`; the non-database-named link disappears from that inventory. |
| + |
| +A replacement containing a regular file instead of the original directory can turn an unconditional problem into an acceptable skipped pair. A bind mount visible in the fleet’s namespace can cause the same mismatch; mount changes confined to the tenant’s namespace cannot. |
| + |
| +`O_DIRECTORY` closes the FIFO/device-open defect. It does not establish directory identity. The revised comment correctly acknowledges that substitution can change whether construction refuses. |
| + |
| +**Smallest acceptable fix:** list entries, inspect them, read links and classify captured targets through the same safely acquired directory descriptor. Resolve the approved systemd root link explicitly when acquiring that root. |
| + |
| +**6. P2 — Header inspection can still cause device-open effects before rejection.** |
| + |
| +**Location:** `internal/ops/sqliteopen_unix.go:26`. |
| + |
| +The walker observes regular file `T/d/watchdog`. T replaces `d` with a link to `/dev`; a privileged header open reaches `/dev/watchdog` on a suitably equipped box. The subsequent regular-file and inode checks cannot undo open-side effects. |
| + |
| +`O_NOFOLLOW` protects the final component. `O_NONBLOCK` and `O_NOCTTY` address particular effects, not ancestor confinement. |
| + |
| +**Smallest acceptable fix:** constrain traversal through pinned directory descriptors before opening the file. Preserve refusal on unsupported platforms. |
| + |
| +**7. P2 — Hibernating aliases and skipped-file claims still hide competing ownership.** |
| + |
| +**Locations:** `internal/ops/litestream.go:381`, `:479`, `:540`, `:709`; `internal/ops/litestream_owners.go:166`, `:241`. |
| + |
| +A live unit claims `/var/lib/carlos-app/u`, reached through its platform state link. Another account’s hibernating exec route names the same database as `/var/lib/private/carlos-app/u/main.db`. The hibernating spelling supplies no candidate, and textual claimant matching misses the shared inode. |
| + |
| +Two unit claims on a tree containing only skipped files, or competing claims on `.cache/app.db` or `main.sqlite`, also bypass listed-file ownership reconciliation. Two hibernating claims can bypass it altogether. |
| + |
| +Exact-spelling live/hibernating conflicts and a unit tree over a listed platform database are reported correctly. |
| + |
| +**Smallest acceptable fix:** reconcile claims by file identity against listed **and skipped** files, retaining claimant information for skipped entries. |
| + |
| +**8. P2 — Required coverage can disappear without an inventory entry.** |
| + |
| +**Locations:** `internal/ops/litestream.go:507`, `:525`; `internal/ops/litestream_owners.go:683`, `:737`. |
| + |
| +Set `CONSOLE_DB` to a missing file outside the globs and route claims. It disappears like the optional console path on a box without a console. “Configured but missing” is not distinguished from “not required.” |
| + |
| +Empty tree-only files and entries disappearing before their first inspection remain deliberate omissions. A database known only to the external replicator, outside these trees and explicit live claims, reaches no list. |
| + |
| +**Smallest acceptable fix:** distinguish required platform databases from optional ones and reconcile against the external replicator’s inventory before cutover. |
| + |
| +**9. P2 — Ordinary tenant changes can refuse replacement replication or hold every account.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:724`, `:729`; `internal/ops/litestream_metered.go:660`, `:880`, `:906`. |
| + |
| +Installing packages or a virtual environment introduces ordinary file and directory links. File links require acceptance; directory links are unconditional problems. The current fleet reports drift, but its replacement refuses. A subsequent ownership change can leave every account waiting for that tenant’s inventory to be resolved. |
| + |
| +A legitimate concurrent rename detected by either inode comparison can also remove an owned file’s facts and hold every recorder. This is conservative refusal, not proof of malicious activity. |
| + |
| +**Smallest acceptable fix:** implement tenant-scoped failures and holds, or explicitly adopt the whole-box policy with an operational recovery procedure before wiring. |
| + |
| +**10. P3 — Route identity remains unbound to independently established filesystem authority.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:176`, `:435`, `:503`, `:572`; `internal/ops/litestream_owners_unix_test.go:381`. |
| + |
| +Route A naming B’s coherent directory and file attributes B’s file to A. The fixture explicitly expects acceptance. |
| + |
| +Root-run units and tenants sharing the platform UID can likewise claim otherwise-unclaimed qualifying platform files. Sharing the UID also makes their links indistinguishable from platform-made links under these checks. This is not acceptance “from anywhere”: modes, links, filesystem boundaries, regular-file status, link count and sidecars still constrain it. |
| + |
| +**Smallest acceptable fix:** bind route claims to independently established roots and identities, with explicit treatment of trusted platform workloads. Deployment approval H remains outside this review. |
| + |
| +**11. P3 — Several tests still permit removal of the behavior they describe.** |
| + |
| +**Locations:** `internal/ops/litestream_metered_test.go:689`, `:807`, `:829`, `:1062`, `:1380`, `:1496`; `internal/ops/litestream_owners.go:194`, `:217`. |
| + |
| +Surviving counterfactuals: |
| + |
| +- Cleared-store and refusing-gate assertions do not establish retained-handle or worker shutdown. Those assertions survive removing `store.Close`. |
| +- Removing the watch’s `lookAgain` call leaves the direct-call file tests intact. |
| +- Removing admission’s pre-witness under-lock stale check is not independently distinguished by the post-witness fixture. |
| +- Removing the latch’s under-lock recheck survives sequential tests. |
| +- Removing `prepareShadowState`’s data-directory check survives the fixture refused earlier by inventory. |
| +- Neither inode comparison has a controlled distinguishing interleaving. |
| +- The 300 ms negative Close assertion can pass before the second goroutine enters Close. |
| +- The clean-look-start test reconstructs time from Unix nanoseconds and depends on wall-clock stability. |
| + |
| +The live-replication test checks positive counts. The asymmetric direct-client assertions at `litestream_metered_test.go:334` and `:337` **do catch swapped fleet recorders**. |
| + |
| +**Smallest acceptable fix:** assert retained-handle/worker shutdown, drive the watch’s file tick, add controlled interleavings, synchronize the second Close caller and use a controlled clock for boundary assertions. Redundant checks need not each receive an artificial mutation test. |
| + |
| +The remaining requested checks have these results: |
| + |
| +| Area | Result at `c9ec11eb` | |
| +|---|---| |
| +| **Tenant-file chain — A/J/P/U** | On stable paths, the start check at `litestream_owners.go:464`, climb at `:493` and stopping-parent check at `:520` reject a distinct tenant’s foreign starting directory, foreign stopping parent and tenant-made links. Every accepting `tenantFile` path reaches the top check at `:508`; earlier refusals need not. A final-component `DBPath` link is refused at `:435`. Approved platform links are resolved before climbing; finding 2 limits their ancestry protection. | |
| +| **Platform UID and root-run units** | Only root and the process UID are equivalent at `:572`. A root/platform file directly inside a distinct tenant’s directory is refused by `tenantFile` at `:490`; a distinct tenant’s file directly inside a platform directory is refused at `:487`. Root-run units stop climbing at platform ownership and can accept qualifying platform files. | |
| +| **New tree refusal — NN2** | `dirUID` at `:538`, `:555` is correct at every depth. The new refusal closes the reported tenant-file/platform-directory case. It does **not** make tree and file checks equivalent generally: `/srv/A/B/a.db`, with A owning the tree and file and B owning the immediate directory, passes a tree check and fails a file check. `litestream_owners_unix_test.go:424` deliberately fixtures this difference. Conversely, a platform-owned tree containing a tenant-owned subdirectory and platform-owned file can pass the tree comparison while failing `tenantFile`’s start check. | |
| +| **Normal platform layouts — P/S/KK2/NN2** | Ordinary DynamicUser state directories have the tenant UID at the resolved root; isolated instances use their own directory chain. The new refusal does not affect those shapes. Exec `dataDir/*.db` claims use `namedFile`; a unit claimant adds `tenantFile`. Protected operator-chosen platform files still pass. A custom platform-owned immediate directory containing a distinct tenant’s file is deliberately refused by tenant file/tree checks pending the stated decision. | |
| +| **Every claimant and one inode — S/W/Z** | `claimChecks` at `:278` retains each distinct root/kind, including two trees, tree-plus-file and unit-plus-exec claims. Every check must pass. Retaining the first successful check’s facts is correct because later successful checks must match its device/inode. The kept-facts duplicate pass at `:217` catches duplicates formed between checks. Stable acceptance is independent of check order; concurrent changes can conservatively refuse and hold everyone. These checks do not form an atomic filesystem snapshot. | |
| +| **Admission — B/I** | `litestream_metered.go:226` rechecks after acquiring the lock; `:240` rechecks after the witness. A file finding during the witness read therefore refuses admission. Staleness immediately after the final atomic load can still precede return or dispatch: finding 3 remains. | |
| +| **Witness retries — B/I** | `:257` makes at most three reads, accepts recovered errors, stops on a witnessed change and treats exhausted errors as uncertainty. Caller cancellation is not mistaken for an ownership change. The five-second contexts do not bound mutex waiting or filesystem `Stat`. An already-running retry sequence can finish reads after a file finding, but its result cannot pass the final stale check. | |
| +| **Registry generation — C/II2** | `registry.go:569`, `:602`, `:612` cover the six consumed columns, insert/delete and rowid immutability. Older-build raw statements fire the same triggers. Same-value updates, unrelated columns and Litestream bookkeeping do not move it. Database-less insertions/deletions and semantically irrelevant changes to the six columns still invalidate conservatively. The host-only raw update at `observe_ownership_test.go:32` distinguishes omission of `host`. | |
| +| **Replacement chain — D/I/J/M** | `chainClosed` at `litestream_metered.go:420` checks a latched predecessor’s own closure first. Window storage, chain detachment and latch publication share `staleMu`; published chain reads take it. Within the documented single-chain contract, replacements cannot open beside open/draining predecessors, and unopened successors cannot latch through them. Inheritance runs once per fleet latch, including intermediate recorders that must outlive Close. Separate or forked chains remain outside that contract. | |
| +| **Close’s final look — M** | Order is store stopped (`:763`) → watch stopped (`:767`) → file look (`:790`) → witness (`:793`) → inheritance (`:799`) → closed (`:800`). A persistent changed/missing owned file on a current fleet holds from its last clean look. Already-stale fleets skip these checks, leaving finding 3. Harmless changes after replication stops can over-hold; filesystem operations can block Close. `closeOnce` prevents another caller from publishing closure during draining. | |
| +| **Lineage — E** | `ownedReplicaKey` at `:965` combines account and cleaned path. Different valid accounts do not share lineage or shadow keys, including after renaming. A→B→A at one path resumes A’s previous lineage; changing app within one account does not create a new key. Separation from an external replicator’s prefix remains unenforced. | |
| +| **Strict walk and reporting — F/K/L/N** | Extensions are recognized case-insensitively; uppercase `.DB` is skipped rather than streamed by tree discovery. Every nonempty regular file not recognized by name receives a header read, including sidecar-shaped names. `sidecarOf` is gone, so a linked base cannot suppress inspection. Unreadable directories, missing/relative unit claims, units without `DBPath`, empty explicit claims and discovered files failing their final look are reported. Findings 7–8 bound completeness. | |
| +| **Explicit claims — N/P/Q** | Live explicit exec claims outside the globs receive ownership checks. The external config path still passes a nil strict walker and retains its previous discovery behavior. Hibernating exec claims outside the candidate set remain absent; alias and skipped-file conflicts remain finding 7. | |
| +| **Leading `..` — K** | `walkWithoutLinks` at `litestream_owners.go:782` permits systemd’s leading `../private/...` shape and rejects ascents after named components. It does not clean away intervening links. Stable accepted traversal agrees within those checked assumptions; ancestry and later substitutions remain separate gaps. | |
| +| **Captured targets — R/BB2/CC2/EE2/GG2** | `Fstatat(fd, dest, …, 0)` follows middle links and resolves `..` physically from the opened directory, including an aliased tree root. Absolute targets use `Stat` unchanged. The second `Readlink` is gone. Non-`ENOENT` target failures report the original path and quoted captured target; ordinary dangling non-database links remain omitted. Finding 5 limits directory identity. | |
| +| **Unsupported platforms — K/EE2** | `linktarget_other.go:17` retains concatenation, but unsupported ownership facts and `sqliteopen_other.go:13` refuse the fleet/header read. The fallback does not enable unrestricted header opening. | |
| +| **Link entries and Accept — T/V/Y/DD2/MM2** | `linkEntry` at `litestream_owners.go:913` distinguishes absolute plain paths and every path/target pair. Relative targets retain literal `Readlink` spelling. Cleaning a plain absolute path cannot forge the prefixed entry. Unchanged accepted pairs remain accepted while classification stays unchanged; target identity is not pinned. The flat impostor fixture distinguishes the old encoding. | |
| +| **Rendering and diagnostics — AA2/FF2/II2/MM2** | `litestream_metered.go:679`, `:686` quote strings, include `Platform` and preserve sections. Reported contents are unambiguous, with list ordering normalized. Private inode facts are intentionally excluded and compared separately. Rendering is compared at `:666` and `:956`, not parsed or used for Accept. `quotedList` keeps fleet-generated plain entries and link pairs unambiguous. The repaired no-recorder path is quoted too. | |
| +| **FIFO test — GG2** | `linktarget_unix_test.go:37` requires `ENOTDIR` in an **open** `PathError`. Restoring the blocking default open times out; dropping only `O_DIRECTORY` produces a stat-stage refusal and fails. It proves open-stage refusal, not identity across listing and reopening. | |
| +| **Fixture budget — HH2/LL2** | The 999-byte target and adaptive 200-byte directory components establish concatenation above 4,096 bytes while the actual file path fits. A 3,200-byte resolved root needs no added component. A root leaving insufficient space fails the new explicit guard. Linked roots are resolved first. This remains a fixture using the Linux pathname limit, not a universal filesystem-limit proof. | |
| +| **Umask, root and CI — H/L/X** | Explicit chmods preserve intended modes under umask `077`; `platformDir` resolves linked temporary roots. Fake ownership still uses basenames and assumes UIDs `44001/44002` differ from the process UID. Root omits unreadable-directory/target cases and skips the root-versus-platform-user distinction. `.amadan/ci.d/10-gate:52` and `Makefile:18` establish commands, not CI’s UID. | |
| +| **Other open items — G/OO2** | A single same-filesystem bind alias remains undetected; two inventoried aliases are rejected by identity. Mount visibility in the fleet’s namespace matters. External-inventory comparison is absent. The pathname gap includes reopens and retained handles; the exit gap includes routine restarts. The twentieth build changes none of those mechanisms. | |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Whatever kind of claim reaches it” needs qualification.** `internal/ops/litestream_owners.go:575` and `litestream_owners_unix_test.go:518` now hold for `tenantFile` and `tenantTree`. They are not universal across `claimKind`: an exec claim on `dataDir/*.db` uses `namedFile`, which intentionally omits these UID restrictions. Scope the wording to tenant file/tree claims. This is an existing documented exception, not a new rejection requirement. |
| + |
| +- **The outside-link fixture still describes concatenation as the implementation.** `internal/ops/litestream_owners_test.go:714` says the spelling is resolved “as concatenated.” Unix now uses `fstatat` against a freshly opened directory. The fixture still distinguishes lexical cleaning correctly; update its implementation description. |
| + |
| +- **The live-replication test’s attribution comment points to incomplete evidence.** `internal/ops/litestream_metered_test.go:921` directs per-database attribution to the lease test, whose final counts are symmetric. The stronger fleet evidence is the asymmetric direct-client assertions at `:334` and `:337`, correctly identified in dispositions `:379`. |
| + |
| +- **“Nothing else moves ownership” would still be too strong semantically.** The current registry comments correctly qualify this by six columns plus insertion/deletion. A database-less route insertion still moves the generation. No change is needed to those corrected comments. |
| + |
| +- **The whole inventory is not serialized.** Rendering is injective over the reported contents, modulo list ordering; it deliberately excludes private inode facts. Those are checked separately. No filename/newline, owner-field or section-boundary collision remains in the reported rendering. |
| + |
| +The twentieth-build owner-comparison comment accurately distinguishes the file-claim race check from the tree-root comparison. The new disposition section correctly leaves billing policy undecided and records the deep items as blocking wiring. Its package-pass statement is an author report; it records no completed full gate for `c9ec11eb`. |
| + |
| +**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 020000d74..3346a2921 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -252,7 +252,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review11.md`, `CODEX-FINDINGS-metered-r |
| | 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 [for a file claim from the nineteenth build in both layouts (before it the climb accepted such a file under a tenant's own tree: review eighteen); for a tree claim from the twentieth (before it the walk accepted it: review nineteen)]. | 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-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 [for a file claim from the nineteenth build in both layouts (before it the climb accepted such a file under a tenant's own tree: review eighteen); for a tree claim from the twentieth, where the platform's directory is the file's immediate directory (before it the walk accepted it: review nineteen). A tenant's file in another tenant's directory inside a tree is still accepted by a tree claim and refused by a file claim, on purpose since review twelve: review twenty]. | 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, 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. | | | |
| @@ -380,4 +380,26 @@ Reports: `OPUS-REVIEW-metered-replicator-review19.md`, `CODEX-FINDINGS-metered-r |
| **Deferred, each blocking wiring, none changed by this build:** Astra 1 (litestream's own opens by path), 2 (replaceable ancestry above a protected directory), 3 (an admitted request delayed past the margin), 4 (uncertainty lost across process exit), 5 (the link's directory resolved again by path: the walk by descriptor), 6 (device effects through an ancestor swap at the header open), 7 (hibernating aliases and claims on skipped files), 8 (required coverage that disappears without an entry), 9 (one tenant's changes refusing or holding the whole box: a decision), 10 (route identity unbound to filesystem authority; approval H's uid binding), 11 (tests that survive removing what they describe: shutdown, watch, interleavings, timing). These need a design (the walk and litestream's opens beneath a pinned root; durable transition state) or a decision (box-wide refusal; approval H), not another build of the walk. Whether CI runs as root is not established by committed source. |
| -Package tests pass under `/var/tmp/ck` and `~/.cache/tmp` in the author's runs, not as root. The full gate on this tip is next; the twentieth build has not been read by a reviewer. |
| +Package tests pass under `/var/tmp/ck` and `~/.cache/tmp` in the author's runs, not as root. The full gate on `c9ec11eb` passed (exit 0, three cross builds, `gate-replicator-c9ec11eb.txt`, 09:52Z) and Amadan CI passed on it. |
| + |
| +## Review twenty (reviewed `c9ec11eb`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-metered-replicator-review20.md`, `CODEX-FINDINGS-metered-replicator-review20.md`. Opus found review nineteen's P3s and Astra 12 closed, the tree-claim fix with a test that fails without it, no new P1 or P2, and three P3s. Astra found the same closed and rejected on the deep items (its 1 to 4 at P1, 5 to 9 at P2, 10 and 11 at P3), unchanged. |
| + |
| +**Bounded changes, made in the twenty-first commit (comments, a fixture and wording; no behaviour changes):** |
| + |
| +| Finding | Disposition | Where | |
| +| --- | --- | --- | |
| +| Opus P3-3: no fixture told the immediate directory's owner from the first component's. | Added: a tenant's file in its own directory beneath a platform directory inside its tree is accepted by both tenant claims. The test is renamed for what it covers. | `litestream_owners_unix_test.go` `TestATenantsFileInAPlatformDirectoryIsRefusedByBothTenantClaims` | |
| +| Claims (both): "whatever kind of claim reaches it" (the platform's own claims ask nothing of the owner); "by construction" (root and the platform's user count as one); the outside-link fixture described as concatenation; the live-replication test pointing at the lease test for attribution; this file's review-eight row. | Corrected in place. | `litestream_owners.go`, the two test files, this file | |
| +| Opus P3-3: the quoting of the no-recorder refusal has no test that fails without it. | Not added in this commit: stated open with Astra 11. | | |
| + |
| +**Raised as decisions, not made as changes** (Opus P3-1 and P3-2: each would change which real boxes refuse their whole build, so it is not a bounded change): |
| + |
| +1. A tenant's file in a platform-owned directory: billable to the tenant or not. Refused by both tenant claims since the nineteenth and twentieth builds; that refusal is fleet-wide (an unowned file cannot be accepted through `Accept`), so a tmpfiles-made `root:foo 0775` directory inside a unit's tree now refuses the box. Whether to bill it, refuse it, or make it acceptable by name is to be decided. |
| +2. Whether a tree claim should ask of every directory inside the tree what a file claim asks of its chain (the root owner's, or the platform's and not writable by group or others). Today a tree claim asks nothing of directories inside the tree (deliberate, fixtured since review twelve); a file claim does. Three layouts differ: another user's directory inside a tenant's tree (that user can drop a sidecar beside the tenant's database, which would be streamed and metered to the tenant); the same higher up; a group- or world-writable platform directory inside a tree. |
| +3. With them, as before: whether one tenant's unaccepted or unowned files refuse the whole box (Astra 9), and approval H's uid binding. |
| + |
| +**Deferred, each blocking wiring, unchanged:** Astra 1 to 11 as listed under review nineteen. These need a design or a decision, not another build of the walk. Whether CI runs as root is not established by committed source. |
| + |
| +The full gate on this tip is next. This commit changes no behaviour and has not been read by a reviewer; the next review round is run with the next change of behaviour. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review20.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review20.md| new file mode 100644 |
| index 000000000..03e596596 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review20.md |
| @@ -0,0 +1,90 @@ |
| +# Review twenty: feat/carloku-metered-replicator-20261001 at `c9ec11eb` |
| + |
| +**Verdict: accept with fixes.** Review nineteen's two Opus P3s and Astra 12 are closed. The tree-claim fix has a test that fails without it. The twentieth build adds no P1 or P2. Its own principle, that the kind of claim must not decide a file's ownership, still fails in three layouts (P3-1). Astra 1–9 and 11, approval H (Astra 10) and the box-wide-refusal decision still block wiring. |
| + |
| +I read source with `git show c9ec11eb:` only and ran nothing. The brief says "review sixteen". As in the last round, I read that as the previous round's findings (review nineteen). The code changes in this build are: |
| +- `litestream_owners.go:536-582`: `dirUID` and the tree-claim refusal. |
| +- `litestream_metered.go:938`: the quoted no-recorder path. |
| +- A comment in `linktarget_unix.go`. |
| +- Two test edits. |
| + |
| +Every area from A to MM2 stands as reviews eighteen and nineteen found it. |
| + |
| +## Review nineteen's findings, one by one |
| + |
| +| Finding | At `c9ec11eb` | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 / Astra claim**: a tree claim accepted a tenant's file in a platform directory | **Closed for that layout** (`litestream_owners.go:580`). Related divergences remain (P3-1 below). | **No.** Without `:580`, `T/bridge/a.db` under tree root `T` passes `:572` (T == T) and is accepted, so `litestream_owners_unix_test.go:541` fails. | |
| +| **Opus P3-2 / Astra 12**: the dead fixture guard, and the weak third check | **Closed.** The guard at `litestream_owners_test.go:769` runs before `mkdirs` and is reachable for a root of 4,084 bytes or more, the only kind that leaves no budget. `:546` asserts `why == ""`. One leftover: the third clause of `:777` now repeats `:769` and is dead. | The guard is a fixture check, not a fix. `:546` is a positive control. It holds under root and under any umask, since owners come through `withOwners` and modes are explicit. | |
| +| **Claim**: "never reaches here" (`:565`) | **Rewritten** (`:568-571`), but slightly loose (claims section). | Wording only. | |
| +| **Claim**: no-recorder path printed raw | **Closed** (`litestream_metered.go:938`). | **Yes.** `litestream_metered_test.go:283` uses `strings.Contains(err.Error(), owned)`, and a plain quoted path contains the raw one. The dispositions claim no test, so this is not a contradiction. | |
| +| **Claim**: "and nothing more" (`linktarget_unix.go`) | **Closed.** It now says a swap can change whether a build is refused. | Wording only. | |
| +| **Astra claim**: per-database attribution listed too broadly | **Closed** (dispositions `:379`). | n/a | |
| +| **Open**: CI as root; the gate on this tip | **Open.** Neither is established. The dispositions say the gate is "next". | n/a | |
| +| **Astra 1–9, 11 (and 10, approval H)** | **Open, unchanged.** Listed as deferred and blocking wiring. | As before. | |
| + |
| +## Findings |
| + |
| +### P3-1. File and tree claims still disagree in three layouts; the new rule closes only one |
| + |
| +**Where:** `litestream_owners.go:539-582` (the tree walk checks only the immediate directory's owner) against `:464-522` (the file claim's start check and climb). |
| + |
| +**`dirUID` is correct at every depth (NN2):** |
| +- For a file directly under the root it is the root's owner. Through a systemd state link it is the real directory's owner, from `os.Stat` at `:396`. |
| +- One level down or deeper, `:555` makes it the owner of the second-to-last component. |
| +- A mutation that kept the first component's owner instead would be caught only by a depth-3 fixture (`T/bridge(root)/sub(T)/a.db`, which both kinds should accept). That fixture does not exist. |
| + |
| +**Where the two kinds still answer differently (each where the file has only one kind of claim):** |
| +1. **Another user's directory in a tenant's tree.** The tree root is `/var/lib/A` (A's), `/var/lib/A/x` belongs to B, and `x/a.db` to A. |
| + - Tree claim: `:572` passes (A == A) and `:580` passes (B is not the platform), so the file is accepted. |
| + - File claim: refused at `:491`. `TestUnitFileChainWithATenantOfItsOwn` pins that refusal; Astra 5 of an earlier round raised it at P2. |
| + - What B gains: B owns the directory holding A's billed database. B can drop a regular `-wal` of its own beside it. The sidecar checks at `:583-597` do not look at the owner, so B's frames would be streamed and metered to A. |
| + - It needs shared write access to A's tree, so reachability is low. |
| +2. **The same with B's directory higher up** (`A/x(B)/y(A)/a.db`). The tree claim accepts, because `dirUID` is A. The file claim is refused at `:520`, because `platformOwned(x)` fails. |
| +3. **A platform directory writable by group or anyone inside a tree.** Units run as root, the tree root is root's, `sub` is root's with mode 0777, and `sub/a.db` is root's. |
| + - Tree claim: accepted. The walk never checks directory modes. |
| + - File claim: refused at `:485`. |
| + |
| +The reverse also happens: a tenant's subdirectory under a root-owned tree root is refused by the tree claim (`:572`) and accepted by a file claim. That direction fails closed. |
| + |
| +**Smallest fix:** in the walk, for `tenantTree`, require each directory passed below the root to be either the root owner's, or the platform's and not writable by group or others (as `platformOwned` checks). Keep `:580` for the immediate directory. Add the B-directory layout (1) to the tree-claim test. |
| + |
| +### P3-2. The new refusal widens the box-wide refusal of Astra 9, and the decision it rests on is not in the decisions list |
| + |
| +**Where:** `litestream_owners.go:580`; `litestream_metered.go:909-911`; dispositions `:381`. |
| + |
| +**Scenario:** |
| +- A tmpfiles rule makes `/var/lib/foo/cache` as `root:foo 0775` inside a unit's tree, and the unit writes `cache/x.db`. |
| +- That file was accepted before this build and is now `Unowned`. |
| +- `Unowned` cannot be accepted through `Accept`. So the whole box's fleet build refuses until the layout changes. |
| + |
| +**Layouts it does not refuse (KK2 and NN2):** |
| +- **DynamicUser state directories:** the dynamic user owns them. |
| +- **Isolated instances:** checked as `tenantFile`. |
| +- **`dataDir/*.db`:** checked as `namedFile`. |
| +- **Units run as root, or sharing the platform's user:** the file is the platform's. |
| + |
| +So the new refusal hits only uncommon layouts. Even so, "refused until decided" is now a fleet-wide policy. Whether a tenant's file in a platform directory is billable is still undecided, and it is not among the deferred decisions at `:381`, which list Astra 9 and approval H. |
| + |
| +**Smallest fix:** add "a tenant's file in a platform-owned directory: billable or not" to the deferred decisions next to Astra 9. |
| + |
| +### P3-3. Test gaps |
| + |
| +- The quoting at `litestream_metered.go:938` has no test that fails without it. A path with a line break, compared exactly, would pin it. |
| +- No depth-3 tree fixture (P3-1). |
| +- The test's name `…RefusedInBothLayouts` now covers three cases. |
| + |
| +## Areas NN2 and OO2 |
| + |
| +**NN2.** `dirUID` is the immediate directory's owner at every depth (P3-1). The rule refuses no systemd, DynamicUser, isolated-instance or activator layout. It does refuse a tenant's file in a root-owned subdirectory that the tenant can write, such as one made by tmpfiles (P3-2). File and tree claims do not yet give one answer in every layout (P3-1, three layouts). Where a file has both kinds of claim, `claimChecks` runs both and one refusal refuses it, so the union is the stricter answer. The order of the checks is not load-bearing. |
| + |
| +**OO2.** Every changed item is listed as a bounded change, and nothing listed as deferred has code changes. The `linktarget_unix.go` comment touches EE2 and Astra 5, but in wording only, and it is listed as such. One thing should have been raised as a decision rather than made as a bounded change: extending "refuse the undecided case" to tree claims changes which real boxes refuse their whole build (P3-2). |
| + |
| +## Claims the code contradicts |
| + |
| +1. **`litestream_owners.go:575-577`** ("refused whatever kind of claim reaches it") and the test's introduction at `litestream_owners_unix_test.go:518-520`: a `namedFile` claim (`dataDir/*.db`) and a `platformDirectory` claim do not check the file's owner at all (no `switch` case, and `:572` and `:580` are tenant-only). A non-platform-owned file in `dataDir/*.db` named by an exec route is accepted. Either narrow the claim to tenant claims, or add an owner check to `namedFile`. Before choosing the second, confirm that no exec layout puts a non-platform-owned file there. |
| +2. **`litestream_owners.go:568-570`** ("whose owner is the file's by construction"): this holds only up to root and the platform's user counting as one owner. A file of the platform's user in a root-owned start directory gets a root with a different uid, and passes through the `platformUID` clause. |
| +3. **Dispositions `:255`** ("a tenant's file in a platform-owned directory is refused … for a tree claim"): true when the platform directory is the file's immediate directory. A file in a tenant's subdirectory under a platform directory is accepted by both kinds, and that is consistent. A tenant's file in another tenant's directory is still accepted by a tree claim (P3-1). |
| +4. **Open, as before:** whether CI runs as root is not established by committed source, and no full gate is recorded on `c9ec11eb`. |
| + |
| +**Verdict: accept with fixes.** P3-1 (the tree walk should check every directory as the file claim's chain does), P3-2 (list the decision) and the claims above are follow-ups. Astra 1–4, approval H and the box-wide-refusal decision still block wiring. |
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go| index f117647a7..54fe41681 100644 |
| --- a/internal/ops/litestream_metered_test.go |
| +++ b/internal/ops/litestream_metered_test.go |
| @@ -920,7 +920,9 @@ func TestMeteredFleetLooksAtTheFilesAgain(t *testing.T) { |
| // |
| // It shows what the pieces above cannot: that the requests litestream makes |
| // 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 |
| +// database's is asserted by the direct-client test above, whose counts |
| +// differ per database, not here and not by the lease package's test, |
| +// whose counts are symmetric); 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 |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index a636065ca..22a161ef5 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -566,17 +566,25 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| // 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). For a file claim the root is the one |
| - // the climb chose, whose owner is the file's by construction, so this |
| - // can fail there only when something changed between the climb and |
| - // the walk; for a tree claim it is the real comparison. |
| + // the climb chose, whose owner is the file's by construction (root and |
| + // the platform's user counting as one there too), so this can fail |
| + // there only when something changed between the climb and the walk; |
| + // for a tree claim it is the real comparison. |
| 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" |
| } |
| // A tenant's file in a directory the platform owns is the undecided |
| - // case, refused whatever kind of claim reaches it: the start check |
| - // refuses it for a file claim, and this refuses it for a tree claim, |
| - // whose walk can pass a platform-made directory inside the tenant's |
| - // tree (review nineteen, Opus P3-1: the kind of claim decided it). |
| + // case, refused by both tenant claims: the start check refuses it for |
| + // a file claim, and this refuses it for a tree claim, whose walk can |
| + // pass a platform-made directory inside the tenant's tree (review |
| + // nineteen, Opus P3-1: the kind of claim decided it). The platform's |
| + // own claims (a named file in the data directory, a platform |
| + // directory) ask nothing of the file's owner and are not this case. |
| + // The two tenant claims still differ on purpose in what they ask of |
| + // the directories inside a tree (a tree claim asks nothing of their |
| + // owners or modes; review twelve): whether they should agree is a |
| + // decision, listed with the deferred ones, because every refusal here |
| + // refuses the whole box's build. |
| if kind == tenantTree && platformUID(dirUID) && !platformUID(facts.UID) { |
| return fileFacts{}, "its directory is the platform's and the file is not" |
| } |
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go| index 74955c894..86d5451c6 100644 |
| --- a/internal/ops/litestream_owners_test.go |
| +++ b/internal/ops/litestream_owners_test.go |
| @@ -713,9 +713,10 @@ func TestStrictTreeNamesALinkToAFile(t *testing.T) { |
| } |
| // A relative target with a ".." after a component that is itself a |
| // link to a directory outside the tree: the kernel steps up from where |
| - // that link led, so the spelling is resolved as concatenated, never |
| - // cleaned (a cleaned join would look beside the link and find |
| - // nothing). (Review fifteen, Opus P3-1.) |
| + // that link led, so the spelling is resolved as written, from the |
| + // link's directory (fstatat on unix), never cleaned (a cleaned join |
| + // would look beside the link and find nothing). (Review fifteen, Opus |
| + // P3-1.) |
| outside := platformDir(t) |
| mkdirs(t, filepath.Join(outside, "o"), filepath.Join(outside, "hidden", "sub"), filepath.Join(outside, "hidden", "dbs")) |
| if err := os.Symlink(filepath.Join(outside, "hidden", "sub"), filepath.Join(outside, "o", "lnk")); err != nil { |
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go| index c00ac6cf1..6603dd04f 100644 |
| --- a/internal/ops/litestream_owners_unix_test.go |
| +++ b/internal/ops/litestream_owners_unix_test.go |
| @@ -516,15 +516,16 @@ func TestOpenRegularNoFollowRefusesALinkToTheSameFile(t *testing.T) { |
| } |
| // A tenant's file in a directory the platform owns is refused whatever is |
| -// above that directory and whatever kind of claim reaches it: a file claim |
| -// under the platform's own tree and under the tenant's own tree, and a |
| -// tree claim on the tenant's tree. Before this the climb decided it one way in each |
| +// above that directory and by both tenant claims: a file claim under the |
| +// platform's own tree and under the tenant's own tree, and a tree claim on |
| +// the tenant's tree. (The platform's own claims ask nothing of a file's |
| +// owner and are not this case.) Before this the climb decided it one way in each |
| // layout (review eighteen, Opus P3-2); the case itself (whether such a file |
| // is billable to the tenant) is not decided, and until it is, it is not. |
| -func TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts(t *testing.T) { |
| +func TestATenantsFileInAPlatformDirectoryIsRefusedByBothTenantClaims(t *testing.T) { |
| const tenant = 44001 |
| top := platformDir(t) |
| - withOwners(t, map[string]uint32{"srv": 0, "T": tenant, "bridge": 0, "a.db": tenant, "own.db": 0}) |
| + withOwners(t, map[string]uint32{"srv": 0, "T": tenant, "bridge": 0, "a.db": tenant, "own.db": 0, "sub": tenant, "deep.db": tenant}) |
| // Under the tenant's own tree: /srv/T/bridge/a.db, bridge the platform's. |
| under := writeFile(t, filepath.Join(top, "srv", "T", "bridge", "a.db")) |
| if _, why := checkOwnedFile(filepath.Dir(under), under, tenantFile); !strings.Contains(why, "its directory is the platform's and the file is not") { |
| @@ -541,6 +542,17 @@ func TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts(t *testing.T) { |
| if _, why := checkOwnedFile(filepath.Join(top, "srv", "T"), under, tenantTree); !strings.Contains(why, "its directory is the platform's and the file is not") { |
| t.Fatalf("a tenant's file in a platform directory, by a tree claim: %q", why) |
| } |
| + // One level further down, in the tenant's own directory beneath the |
| + // platform's: the immediate directory is the tenant's, and both claims |
| + // accept (review twenty, Opus: no fixture told the immediate |
| + // directory's owner from the first component's). |
| + deep := writeFile(t, filepath.Join(top, "srv", "T", "bridge", "sub", "deep.db")) |
| + if _, why := checkOwnedFile(filepath.Join(top, "srv", "T"), deep, tenantTree); why != "" { |
| + t.Fatalf("a tenant's file in its own directory beneath a platform directory, by a tree claim: %q", why) |
| + } |
| + if _, why := checkOwnedFile(filepath.Dir(deep), deep, tenantFile); why != "" { |
| + t.Fatalf("the same file by a file claim: %q", why) |
| + } |
| // The platform's own file in its own directory is still the platform's. |
| own := writeFile(t, filepath.Join(top, "srv", "bridge", "own.db")) |
| if _, why := checkOwnedFile(filepath.Dir(own), own, tenantFile); why != "" { |