amadan
Close round eleven: a record's provisioned bucket is its bucket, a grant is counted before it touches IAM, a rotation compares the whole estate
Round eleven (Astra reject: one P1, two P2; Opus accept with fixes: one P2, one P3) found that a grant over an older record provisioned on one bucket could choose the minted name instead, leaving two buckets behind one record; that a waiting rotation ignored a grant that ran meanwhile; and that the user and its policy were written before the grant was counted. - Grant: the record's bucket is the one it is provisioned on, else the one its grant block names. The arguments choose only for a store that has none. - Grant and Teardown refuse a record that names two buckets (ErrTwoBuckets) before they lease or call the provider. - A teardown note for a bucket that is not the record's refuses the grant with ErrUnfinishedTeardown, which gives its lease back. - GrantBlock.Walks is written before EnsureUser. A healthy re-grant now writes its record once. - Both rotation verbs compare the whole estate under the lease. Each fix has a reproduction and was mutation-checked in a scratch copy. The round's reports, its dispositions and the round twelve brief are added. OUTSTANDING.md is synchronized with the side branches' real stages and gains 19g (two refusals whose door copy is wrong). No review round has passed. Billing is off; nothing attests.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
10 files changed,
+685
−87
docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md+19 −0docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round11.md+100 −0docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round12.md+28 −0docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round11.md+97 −0implementation/carloku-paid-launch/AGENT_HANDOFF.md+2 −2implementation/carloku-paid-launch/OUTSTANDING.md+24 −15internal/objstore/objstore.go+7 −6internal/objstore/ownership.go+12 −0internal/objstore/ownership_test.go+298 −16internal/objstore/walk.go+98 −48
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md| index e52b3622b..c77fe11c8 100644 |
| --- a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md |
| +++ b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md |
| @@ -368,3 +368,22 @@ Astra: three P1. Opus: one P2, one P3. Astra's third and Opus's first are the sa |
| The walk count is written with the same tolerance the grant's last write has: if the console's declare door wrote the record in between and the estate is unchanged, the count goes on the record as it now stands (`TestGrantRetriesLostCAS`). |
| Mutation checks in a scratch copy, each failing its test: the walk not counted; a take-back allowed to carry a note to another bucket; leaving stores counted as holders of an unsupported name. Storage simulation: 3,000 seeds on this code. |
| + |
| +## Round eleven (reviewed `84311b9f`; Astra rejected, Opus accepted with fixes) |
| + |
| +Reports: `CODEX-FINDINGS-carloku-paid-launch-overnight-round11.md`, `OPUS-REVIEW-carloku-paid-launch-overnight-round11.md`. |
| + |
| +Astra: one P1, two P2. Opus: one P2, one P3. Astra's F1 and Opus's N1 are the same fault. Nothing that attests, reaches Stripe with a switch unset, or makes an open late-request defect reachable. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra F1, Opus N1: a grant over an older record provisioned on X chooses the minted name Y, creates it, and a later teardown destroys X and deletes the record while Y stands; or the take-back forgets a bucket left to another account | Accepted, closed at the root. Round ten refused one shape of it (a teardown that had noted a deletion). A record's bucket is now the one it is provisioned on, else the one its grant block names, in the order Teardown reads them; the arguments choose a bucket only for a store that has none. A grant cannot move a store to another name, so there is no second bucket. Such a grant block is no longer written as minted. | `walk.go` `Grant` (`bucketFor`) | `TestGrantOverATeardownKeepsTheRecordsOwnBucket`, `TestGrantOverARetainedBucketTakesThatBucketBack`. They replace round ten's `TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother`, whose premise (the grant is for Y) no longer holds. | |
| +| Astra, claims: at the final record delete everything the record named is gone, yet a record can name X and Y | Accepted. The root fix stops this build writing such a record. One written by an earlier build is refused by Grant and by Teardown before they lease, call the provider or write (`ErrTwoBuckets`). A person decides; there is no operator verb for it (`OUTSTANDING.md` 19g). | `walk.go` `oneBucket` | `TestWalksRefuseARecordThatNamesTwoBuckets` | |
| +| Astra F2: a waiting rotation ignores the grant count and reaps the key a failed grant delivered | Accepted. Both rotation verbs compare the whole estate under the lease, as teardown and grant do. Opus read the same schedule as Rotate's documented reap; the stricter reading is taken. | `walk.go` `leaseForRotation` | `TestRotationStopsWhenAGrantRanWhileItWaited` (both verbs; the grant really delivers a key the record does not name) | |
| +| Astra F3: the user and its policy are written before the count | Accepted. The count is written before `EnsureUser`. Every grant that reaches the provider is counted, a healthy re-grant included, so that one now writes its record once (round ten said it wrote nothing). | `walk.go` `Grant`; `objstore.go` | `TestGrantThatCannotBeCountedTouchesNothing` (a first grant and a healthy re-grant, each failing at its count: no user, no policy write, no bucket) | |
| +| Opus N1 as written: refuse a grant whenever an unfinished teardown's coordinates differ from the grant's bucket, note or no note | Applied, and kept under the root fix. It is now reachable only when a teardown note names a bucket that is not the record's own, which no walk writes. | `walk.go` `Grant` | `TestGrantIsRefusedOverATeardownNoteForAnotherBucket` | |
| +| Opus N2: the refusal is typed as a conflict, holds the name for thirty minutes, and the door says to run the grant again | Partly fixed. The refusal has its own sentinel (`ErrUnfinishedTeardown`), gives its lease back, and makes no provider call. The door's sentence is unchanged: new copy needs approval. Open as 19g. | `ownership.go`, `walk.go` `walkEndedClean` | the same test | |
| +| Astra, claims: the handoff says an unsettled operation is acknowledged only after its key was tried at the provider | Corrected: a resolved review stands in for the fence, on a person's word. | `AGENT_HANDOFF.md` item 8 | none | |
| +| Both, claims: "every grant that creates anything is counted before it calls the provider"; "a walk requires the same estate" | Made true by F3 and F2, and the handoff reworded. | `AGENT_HANDOFF.md` item 5, `objstore.go` | none | |
| + |
| +Mutation checks in a scratch copy, each failing its test: the provisioned bucket not preferred; the grant block written as minted; rotation comparing bucket and key only; the count moved after `EnsureUser`; the teardown-note refusal off; the two-bucket refusal off in Grant, and off in Teardown. Storage simulation: 3,000 seeds on this code. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round11.md b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round11.md| new file mode 100644 |
| index 000000000..fdb4da216 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round11.md |
| @@ -0,0 +1,100 @@ |
| +# Round-eleven review: `84311b9f` |
| + |
| +Reviewed against base `aed42815`, concentrating on `94ca7482..84311b9f`. Static review only: no modifications, tests, network commands, Stripe CLI or AWS CLI. Test counterfactuals below are based on reading. |
| + |
| +All references identify the pinned commit. **O** = `internal/objstore`, **C** = `internal/console`, **B** = `internal/billingstripe`, **D** = `implementation/carloku-paid-launch`, **S** = `docs/superpowers/specs`. |
| + |
| +## 1. Round-ten dispositions |
| + |
| +| Finding | Disposition | Fix and test | Would the test pass without the fix? | |
| +|---|---|---|---| |
| +| Astra F1: ordinary failed-grant retry leaves the waiting teardown’s estate unchanged | **Closed for the reported schedule.** The broader write-before-provider claim remains false; F3 below. | Count persisted at `O/walk.go:444`, included in identity at `:1168`, compared by teardown at `:846`. Regression: `O/ownership_test.go:2489`. | **No** if the increments are removed while preserving the writes. Removing the entire counting block also breaks the fixture’s assumption that the final write is its second write (`:2508`), so that mutation’s failure alone does not establish the intended schedule. | |
| +| Astra F2: take-back copies X’s deletion onto Y’s ledger | **Closed for the reported deletion-note replay.** A take-back before deletion is recorded still permits a second bucket; F1 below. | Explicit bucket and legacy fallback at `O/walk.go:234`; mismatch refused at `:247`; replay at `:277`; deletion records its bucket at `:1019`. Regression: `O/ownership_test.go:2548`. | **No** without the mismatch refusal: Y’s ledger receives X’s timestamp. The test also requires the explicit `DeletedBucket`; it does not exercise the legacy fallback with that field absent. | |
| +| Astra F3 / Opus N1: unsupported-name holders retain each other forever | **Closed for the reported mutual hold.** | Unsupported-name enumeration excludes leaving records at `O/walk.go:926`, `:1249`. Extended regression: `O/ownership_test.go:2404`, both departures and final retry at `:2444`. | **No.** Counting the first retained record again makes the second foreign-account teardown return `ErrBucketRetained`. | |
| +| Opus N1, scanner-debt remainder | **Accepted and left open**, as dispositioned. | `D/OUTSTANDING.md:59`; no fix or regression. | Not applicable. | |
| +| Opus N2: late purge refusal gives false copy and leaves estate inaccessible until restart | **Accepted and left open.** Documentation corrected; behavior unchanged. | `D/OUTSTANDING.md:60` now states both limitations. The member copy remains at `C/ownership.go:571`. No new regression. | Not applicable. | |
| + |
| +### Round-ten claims |
| + |
| +| Claim | Disposition and evidence | Test / counterfactual | |
| +|---|---|---| |
| +| Unsupported-name teardown decides through the ownership ledger | **Closed as wording.** `D/AGENT_HANDOFF.md:35` now identifies record enumeration. | Behavioral regression above; no documentation test. | |
| +| Unsupported-name retained teardowns cannot complete | **Closed for the reported two-holder schedule.** | `O/ownership_test.go:2444` fails with leaving records counted. Scanner debt remains explicitly open. | |
| +| Failed leases have no same-app retry exception | **Closed as wording.** The exception appears in `D/AGENT_HANDOFF.md:35`; `O/fence.go:39` now distinguishes bucket/user overlap from the separate cross-name user defect. | No new behavioral regression; the exception remains implemented at `O/walklock.go:155`. | |
| +| The late-purge estate is immediately reachable; “Nothing was deleted” | **Documentation closed; member copy accepted and left open.** `D/OUTSTANDING.md:60` is accurate about the reported case; `C/ownership.go:573` remains false for it. | No new regression. | |
| +| Every unconfirmed acknowledgement consumes the provider key | **Partly closed.** The evidence introduction now states the human-resolution exception (`S/2026-10-01-stripe-idempotency-evidence.md:9`); the handoff’s unconditional sentence remains (`D/AGENT_HANDOFF.md:38`). Code explicitly substitutes a resolved review at `C/billingquantity.go:745`. | No documentation regression. | |
| +| The canceled-subscription check establishes an unchanged quantity despite a failed reread | **Closed in the procedure.** `B/sandbox_fence_test.go:421` now requires a successful, single-item reread before comparing quantity. | **Yes**, a normal successful sandbox run could pass without this guard. No injected reread-failure regression was added. The procedure was not run. | |
| +| Every waiting walk recognizes the same estate; every creating grant counts itself before provider calls | **Partly closed.** Teardown recognizes the new count. Rotation ignores it, and IAM creation precedes it; F2 and F3 below. | Existing rotation regression changes the recorded key (`O/ownership_test.go:1906`); it does not test an intervening grant whose final write fails. | |
| + |
| +## 2. New findings |
| + |
| +### F1. P1 — A cross-name take-back before bucket deletion can leave a live bucket behind a successful teardown |
| + |
| +**Where:** `O/walk.go:176`, `:234`, `:253`, `:460`, `:495`, `:868`, `:1148`. |
| + |
| +**Scenario:** |
| + |
| +1. Start with the same supported legacy shape used by the new regression: `Declared` and `Provisioned` naming X, with no `Grant`. The deployment’s minted name for this app is Y, different from X. |
| +2. Teardown records its block and X’s destroying marker, then fails at `EmptyBucket`. No deletion timestamp exists. |
| +3. After the lease permits it, Grant takes the store back. `bucketFor` ignores `Provisioned` and chooses Y. The new cross-name refusal runs only when `BucketDeletedAt` is nonzero, so it does not apply. |
| +4. Grant persists `Grant.Bucket=Y`, counts the walk, creates Y, and delivers Y’s credentials. Its final store-record write fails before taking effect. |
| +5. The persisted record now names Y in `Grant` and X in `Provisioned`. The app can write objects into Y. |
| +6. A fresh teardown chooses X from `Provisioned`, destroys X, deletes the deterministic user, removes the delivered configuration, and deletes the store record successfully. Its final guard accepts the unchanged old key and `Grant.Bucket=Y`. |
| +7. Y and its objects remain, with an open ownership interval but no store record for the teardown to follow. |
| + |
| +This does not require a late provider request or clock disagreement. The new refusal protects a recorded deletion, but not an unfinished teardown that has not reached deletion. |
| + |
| +**Smallest acceptable fix:** Refuse a cross-name take-back whenever the unfinished estate names another bucket, regardless of its deletion timestamp, or preserve the provisioned bucket as the grant’s target. Add this legacy-X/Y schedule with failure of the grant’s final write; teardown must not report success while Y survives. |
| + |
| +### F2. P2 — A waiting rotation ignores the grant count and reaps the newly delivered key |
| + |
| +**Where:** `O/walk.go:445`, `:473`, `:481`, `:495`, `:614`, `:704`, `:719`. |
| + |
| +**Scenario:** |
| + |
| +1. A provisioned store records key K1. Rotation R reads that record and pauses before acquiring its lease. |
| +2. A grant repairs missing delivered configuration. It increments `Grant.Walks`, reaps K1, mints K2 and delivers K2. |
| +3. The grant’s final record write fails. The record still names K1 and has no teardown block, but its walk count has increased. K2 is the working delivered credential. |
| +4. After the failed grant’s lease expires, R acquires the lease and rereads the record. |
| +5. `leaseForRotation` compares only bucket, K1 and the absence of teardown. It ignores the changed count and accepts the record. |
| +6. `ReapKeys(..., []string{K1})` deletes K2. If the subsequent mint or delivery fails, the app retains a credential that no longer works. Even a successful delivery does not preserve the overlap promised to processes still using K2. |
| + |
| +**Smallest acceptable fix:** Compare the captured estate, including `Grant.Walks`, in `leaseForRotation`. Extend `TestRotationStopsWhenTheStoreWasReplacedWhileItWaited` with an intervening grant whose final write fails, asserting refusal before any key deletion. |
| + |
| +The existing test at `O/ownership_test.go:1906` changes the recorded key, so it cannot detect this omission. |
| + |
| +### F3. P2 — IAM creation and policy delivery happen before the count is durable |
| + |
| +**Where:** `O/walk.go:417`, `:428`, `:445`; `O/aws.go:434`, `:449`. |
| + |
| +**Scenario:** |
| + |
| +1. A failed grant has already persisted its grant block and ownership interval, but failed before creating its IAM user. It has neither `Provisioned` nor `Teardown`. |
| +2. A retry calls `EnsureUser` at `:417`. That creates the principal and writes its inline policy. |
| +3. The count write at `:446` fails before taking effect. |
| +4. Provider authority has been created while the persisted estate remains identical to the pre-retry estate. |
| + |
| +The healthy-looking path also rewrites the inline policy before returning at `:434`, without ever reaching the count. This can restore access when the policy is missing, including the state documented in open item 19b. |
| + |
| +This is a concrete exception to the proposed write-before-provider mechanism. The first schedule alone does not establish data loss, but the count cannot identify every grant that created or restored authority. |
| + |
| +**Smallest acceptable fix:** Persist the attempt generation before `EnsureUser`. If healthy re-grants must avoid record writes, they must also avoid provider mutations unless those mutations receive their own persisted generation. Add a regression that fails the generation write and asserts that no IAM creation or policy write occurred. |
| + |
| +### Other requested checks |
| + |
| +For coherent same-bucket records, the count’s conflict retry compares the fresh estate with the **pre-increment** estate before incrementing the fresh record (`O/walk.go:444`). I found no additional count transplant through that retry, or permanent refusal caused by the count alone: a new invocation reads the new count. |
| + |
| +The unsupported-name branch continues to count records without a teardown block. I found no new schedule in which that exclusion alone destroys a coherent holder’s bucket before its owner has requested teardown. The accepted scanner-debt limitation remains open. |
| + |
| +The reported X-to-Y deletion-note replay is blocked. I found no additional reachable wrong-ledger replay from a coherent record; F1 concerns a second bucket created before any deletion note exists. |
| + |
| +## 3. Claims the code contradicts |
| + |
| +| Location | Claim | Contradiction | |
| +|---|---|---| |
| +| `D/AGENT_HANDOFF.md:35`; `O/objstore.go:64`; dispositions `:359` | Every grant that creates anything is counted before calling the provider. | `EnsureUser` creates a principal and writes policy before the count; F3. | |
| +| `D/AGENT_HANDOFF.md:35`; `D/OUTSTANDING.md:86` | A walk rereads the record and requires the same estate. | Rotation accepts a changed grant count with the old recorded key; F2. The handoff accurately lists rotation’s narrower comparison, but that comparison does not establish the broader claim. | |
| +| `O/walk.go:1147` | At final record deletion, everything the record named is gone. | In F1, `Grant` names Y while teardown destroys only X from `Provisioned`; Y survives record deletion. | |
| +| `D/AGENT_HANDOFF.md:38` | An operation nobody can show ran is acknowledged only after its key has been tried at the provider. | Human resolution can substitute without a provider fence (`C/billingquantity.go:658`, `:745`). The evidence document is corrected; this handoff sentence is not. | |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round12.md b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round12.md| new file mode 100644 |
| index 000000000..ce0f98c8b |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round12.md |
| @@ -0,0 +1,28 @@ |
| +# Review brief, round 12: Carloku paid launch, overnight changes of 2026-10-01 |
| + |
| +Read the round 1 brief (`CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight.md`) for the fixed rulings and the output rules, the round 3 to 11 briefs for rulings 6 to 12, then `CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md` (its last section covers round 11) and `implementation/carloku-paid-launch/OUTSTANDING.md`. |
| + |
| +Review the commit named in the prompt. Read source only with `git show <commit>:<path>`. The changes since round 11 are `git diff 84311b9f <commit>`. The code changes are in `internal/objstore` only (`walk.go`, `ownership.go`, `objstore.go`, `ownership_test.go`). The rest is documents. |
| + |
| +Still the fixed snapshot: round 11's findings, nothing else. Nothing may attest. The quantity worker has two switches, both unset. Billing is off. |
| + |
| +Do not report again, as findings, what `OUTSTANDING.md` lists as open: items 6 and 6a, 7 and 9, 19a to 19g. Do report anything that makes one of them worse than the document says, and any claim about them the code contradicts. |
| + |
| +## First: a disposition table |
| + |
| +For every round 11 finding (Astra F1 to F3 and the claims; Opus N1, N2 and the claims): closed, partly closed, not closed, or accepted and left open as the dispositions say. File and line of the fix and of the test, and whether the test would pass without the fix. |
| + |
| +## Then: new findings only |
| + |
| +- **One bucket per record** (`Grant`'s `bucketFor`, `oneBucket`). The claim: a record's bucket is the one it is provisioned on, else the one its grant block names; the arguments choose only for a store that has none; so no walk of this build leaves a store's objects or credentials in a bucket its record does not lead to. Find a schedule that ends with a live bucket, user, policy or key that no record names. Find a caller that passes an adopted bucket and now silently gets another, where that is a harm rather than the documented "a re-run never re-chooses the bucket". Say whether `Minted` can now be written true for a bucket this grant did not mint, or false for one it did, and what either would let the ownership ledger claim. |
| +- **The two refusals** (`ErrTwoBuckets`, `ErrUnfinishedTeardown`). Find a record this build can write that then hits either. Find a path (purge, declaration-only teardown, rotation, scanning doors) that still acts on a record naming two buckets. |
| +- **The count before the user** (`GrantBlock.Walks` written before `EnsureUser`). Find a provider call of Grant that precedes it. A healthy re-grant now writes the record: find a door or loop that calls Grant on a timer or per request and would now churn the record, conflict with the declare door, or refuse waiting walks for good. |
| +- **Rotation's estate compare** (`leaseForRotation`). Find a legitimate rotation that is now refused on every attempt. Find a schedule where a rotation still reaps a key a grant delivered. |
| +- **Claims** in `AGENT_HANDOFF.md` items 5 and 8, `OUTSTANDING.md`, and the round 11 dispositions. |
| + |
| +## What to return |
| + |
| +1. The disposition table. |
| +2. New findings, most severe first, in the round 1 format. |
| +3. Claims the code contradicts. |
| +4. A one-line verdict: accept, accept with fixes, or reject. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round11.md b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round11.md| new file mode 100644 |
| index 000000000..20590862e |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round11.md |
| @@ -0,0 +1,97 @@ |
| +# Opus production and authority review, round 11: Carloku paid launch, `aed42815..84311b9f` |
| + |
| +This was a read-only pass using `git show` only. I changed nothing, ran no tests and made no network calls. Every "fails without the fix?" answer comes from reading the test against the code. The code change since round 10 (`94ca7482..84311b9f`) is in `internal/objstore` (`walk.go`, `objstore.go`, `fence.go`), `ownership_test.go`, and one check in `internal/billingstripe/sandbox_fence_test.go`. The rest of the diff is documents. |
| + |
| +Abbreviations: **O** = `internal/objstore`, **C** = `internal/console`, **B** = `internal/billingstripe`, **D** = `implementation/carloku-paid-launch`. |
| + |
| +**Production authority** |
| + |
| +- **Nothing attests.** `SetAttestation` and `SetAllClientsInstrumented` (`usage/durabilityledger.go:1455`, `:1475`) still have no non-test caller. |
| +- **The quantity worker cannot reach Stripe with either switch unset.** `C/billingquantity.go`, `C/billingservice.go`, `B/client.go` and `cmd/console/billing.go` did not change this round. `cmd/console/billing.go:40` still accepts only `stripe-sandbox-verified`. My round 9 and 10 reading of the send lease, the key fence and the second switch still holds. |
| +- **The sandbox test** is still skipped unless `CARLOS_STRIPE_SANDBOX_KEY` is set (`B/sandbox_fence_test.go:46-48`). It refuses any key that is not `rk_test_` (`:50-52`). It has not been run. |
| +- **Wrong billing or leaks.** I found no path that bills a customer wrongly. No new log line or error carries a secret or a Stripe identifier. The new Grant refusal names bucket names in an error that the doors log and do not return (`C/apistore.go` grant door). |
| +- **Customer data.** I found no new path that destroys data a store still holds. I found one path where a take-back forgets a bucket it left behind (N1). |
| + |
| +## 1. Disposition table (round 10) |
| + |
| +| Finding | Status | Fix | Test; fails without the fix? | |
| +|---|---|---|---| |
| +| Astra F1: a failed grant retry creates and delivers, and the record still compares equal | **Closed** | `O/walk.go:437-458` (`GrantBlock.Walks` written before `EnsureBucket`); `estateOf` `:1168`; `O/objstore.go:64-70` | `TestStaleTeardownDoesNotDestroyWhatAFailedGrantRetryMade` (`O/ownership_test.go:2489`). **Fails**: without the count, the retry writes nothing before its failing last write, and the stale teardown destroys `invoice.pdf`. | |
| +| Astra F2: a take-back records one bucket's deletion on another bucket's ledger | **Closed for the reported shape** (a teardown that noted a deletion). The same shape without the note still takes the store to another name (N1). | `O/walk.go:231-250`, `:277-280`; `DeletedBucket` written at `:1019`; `O/objstore.go:170-174` | `TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother` (`:2548`). **Fails**: Y's `DestroyedAt` gets set. It cannot tell `DeletedBucket` from the `Provisioned.Bucket` fallback, which gives the same answer here. | |
| +| Astra F3 / Opus N1: two accounts sharing an unsupported name each wait on the other | **Closed** | `storeHolders(…, false)` at `O/walk.go:926`; filter at `:1249` | Extension at `O/ownership_test.go:2440-2457`. **Fails**: the second teardown returns `ErrBucketRetained`. | |
| +| Opus N1, second effect: no scanner debt for an unsupported name | **Accepted, left open as 19f.** The "latent" claim holds. The only production scanner is ClamAV, and its `EnsureDisabled` does nothing (`O/clamav.go:96`). `guardduty` exists only as an engine string (`O/objstore.go:104`). | — | — | |
| +| Opus N2: the late purge refusal says "Nothing was deleted" and leaves the estate unreachable | **Accepted, left open.** The 19e wording now matches the code. The member copy is unchanged, pending approval. | `D/OUTSTANDING.md:60` | — | |
| +| Opus round 10 claims (`fence.go`, the evidence document's opening, the sandbox reread) | **Closed** | `O/fence.go:39-46`; evidence document `:9`; `B/sandbox_fence_test.go:421-426` now fails if the reread fails | Not applicable. | |
| + |
| +## 2. New findings, most severe first |
| + |
| +### N1. P2: a take-back to another name over a teardown with no deletion note forgets the bucket it left |
| + |
| +**Where:** |
| +- The refusal applies only when `BucketDeletedAt` is set (`O/walk.go:234`). |
| +- `bucketFor` ignores `Provisioned` (`:176-185`). |
| +- A successful take-back sets `Teardown = nil` (`:494`). |
| + |
| +**Scenario:** |
| +1. A legacy record for account X, app A, has `Provisioned.Bucket = X` and no `Grant` (the shape Astra F2 used). Another account's store C also names X. |
| +2. A's teardown writes its block. It finds C as a holder, so it leaves X, deletes A's user, and returns `ErrBucketRetained`. `BucketDeletedAt` is still zero. |
| +3. A's member grants again. `bucketFor` picks the minted name Y, and `deletedBucket` is empty, so nothing refuses. |
| +4. The take-back creates Y, delivers Y's credentials, writes `Provisioned = Y` and clears the teardown block. |
| + |
| +**Result:** |
| +- A's objects stay in X, and C can still read them. |
| +- No record names that retention any more. `ErrBucketRetained`, the only "a person decides" signal, is gone. |
| +- The comment at `:243-246` names exactly this harm ("leave the old name's teardown unfinished behind a record that no longer names it"). The grant door then answers success. |
| +- The same happens when the teardown failed after `beginBucketTeardown` marked X as destroying but before `EmptyBucket`. X is then left with its data, billing, with no record naming it, and its ledger is stuck on "being destroyed by A". |
| + |
| +**Scope:** only records whose teardown coordinates differ from `bucketFor`, which current grants never write. Whether such legacy records exist cannot be determined from the repository. |
| + |
| +**Smallest fix:** when `rec.Teardown != nil`, refuse whenever the teardown's coordinates (`Provisioned.Bucket`, else `Grant.Bucket`) differ from `bucket`, whether or not a note was written. Add the retained X/Y case to `TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother`. |
| + |
| +### N2. P3: the new refusal is typed as a CAS conflict, and the door tells the operator to re-run the grant, which never works |
| + |
| +**Where:** `O/walk.go:248` wraps `release.ErrCASConflict`. The grant door (`C/apistore.go:633-643`) answers "re-run the grant to resume". `apistore_provision.go` answers "re-run `carlos store create`". |
| + |
| +**Scenario:** |
| +1. A record has a teardown note for bucket X, and the grant resolves to Y. |
| +2. Every re-run of the grant is refused the same way. |
| +3. The way forward is a teardown, and no door says so. The refusal is also not `walkEndedClean`, so the grant lease on Y stands for 30 minutes. That does not block the teardown, which leases X. |
| + |
| +**Smallest fix:** add a sentinel such as `ErrUnfinishedTeardown`. Add it to `walkEndedClean`, and give it a door message that says to finish the destroy first. |
| + |
| +### Asked and not found |
| + |
| +- **A provider call before the count.** |
| + - `EnsureUser` (`O/walk.go:417`) creates the user and writes its policy before the count. It does this only on a record with no teardown block, because a take-back has already counted `TakeBacks` at `:282`. It mints no key and delivers nothing. |
| + - A waiting teardown that then proceeds deletes a keyless user of the very store it was asked to destroy. |
| + - The probes (`BucketExists`, `BucketGrantees`), `openBucketOwner` and `markBucketDestroyed` are reads or ledger writes. `EnsureBucket`, `ReapKeys`, `MintKey` and `writeEnv` all follow the count. |
| + - The claim is wrong in its wording only (§3). |
| +- **The count refusing a legitimate walk for good.** No. Each comparison is between two reads of one invocation. A lost-response `Put` that did apply makes the fresh estate differ, so that invocation fails and a re-run reads afresh. A grant that keeps failing bumps `Walks` each time, and waiting teardowns lose one invocation each, but none is refused for good. |
| +- **The count's conflict retry on another estate.** |
| + - `before` is taken before the increment, and the retry requires `Grant != nil` and an equal estate (bucket, key, grant instant, walks, teardown state, deletion flag, take-backs). |
| + - A late final `Put` from an earlier attempt (the same-kind lease exception) changes the key and is refused. A purge cannot delete a record that carries a grant. |
| + - The fields the retry does not compare are `Declared` and `Scanning`. The declare door can change the env prefix, and `deny` was computed from the earlier read. Both are this same estate, as in the existing final-write retry. |
| +- **Unsupported names destroyed under a store that has not asked to go.** |
| + - For such a name, Grant refuses at `:195` whenever `Grant.Bucket` is the name, so a teardown block there always means the owner asked for destruction and cannot be undone. |
| + - The only exception is the legacy shape with only a `Provisioned` block, whose take-back moves to the minted name anyway (N1). |
| + - Teardowns of one name serialize on the hashed lease. |
| + - The re-run after the last holder has destroyed the bucket treats `NoSuchBucket` as success (`O/aws.go:335`, `:368`). AWS no longer creates names this build refuses. |
| +- **A record that can never be granted and never be torn down.** None found. A refused take-back leaves the note in place. The teardown re-run then takes the `bucketDone` path (`:942`, or straight to `DeleteUser` for an unsupported name), closes, and deletes the record. |
| +- **Rotation.** |
| + - It compares bucket, key and the absence of a teardown (`O/walk.go` rotation guard, about `:719`). |
| + - A take-back either leaves the teardown block (failure) or changes the key (success). A grant retry that delivered a new key and failed at its last write leaves the key equal, so a waiting rotation reaps that key and delivers its own. That is the same store, and it is Rotate's documented reap of unrecorded keys. |
| + - A rotation cannot act on a store other than the one it was asked about. |
| + |
| +## 3. Claims the code contradicts |
| + |
| +| Location | Claim | What the code supports | |
| +|---|---|---| |
| +| `D/AGENT_HANDOFF.md` item 5; `O/objstore.go:64-66` | "Every grant that creates anything is counted on the record before it calls the provider." | `EnsureUser` creates the user before the count (`O/walk.go:417` vs `:445`). It is harmless (no key, no teardown block), but the sentence should say "before it creates the bucket or mints or delivers a key". | |
| +| Dispositions, round 10 Astra F2 row (`:360`) | "A grant for another name over an unfinished teardown is refused until that teardown is finished." | Refused only when that teardown noted a deleted bucket (`O/walk.go:234`). This is N1. | |
| +| Grant door copy (`C/apistore.go:641`) | "nothing unrecorded was left behind; re-run the grant to resume" | Re-running never resumes after the new refusal (N2). After N1's take-back, the left bucket is unrecorded. | |
| + |
| +Nothing in `walklock.go`, `fence.go` (now accurate), the evidence document, "The fence" in `billingquantity.go`, or `OUTSTANDING.md` items 6, 6a, 7, 9 or 19a–19f is contradicted by the code. Nothing this round makes any of them worse. |
| + |
| +## 4. Verdict |
| + |
| +**Accept with fixes.** Nothing attests, the worker cannot reach Stripe with either switch unset, billing is off, and all three round 10 findings are closed for their reported shapes. N1 (extend the refusal to every teardown whose coordinates differ) and the two wording fixes should land before the storage claims are relied on. |
diff --git a/implementation/carloku-paid-launch/AGENT_HANDOFF.md b/implementation/carloku-paid-launch/AGENT_HANDOFF.md| index fdf7fe6a7..9fe01bb90 100644 |
| --- a/implementation/carloku-paid-launch/AGENT_HANDOFF.md |
| +++ b/implementation/carloku-paid-launch/AGENT_HANDOFF.md |
| @@ -32,10 +32,10 @@ When a review is approved, apply each string byte for byte: the site by replacin |
| 2. Console GET `/a/{sqid}/billing` (`billingpage.go`, `templates/billing.html`): current plan, owner-only chooser posting to the existing checkout door, `?country=` price preview computed from the plans terms, error and pending banners, portal button once a customer exists, read-only for members, byte-identical 404 for non-members, closed state when billing is off. `plans.Countries()` exposes the embedded country list. |
| 3. Durability ledger: the epoch read race is fixed (epoch and created-at are immutable after recover) with a `-race` regression. An unmetered replication fallback on a box whose ledger opened now files an escape (`DurabilityLedger.Escaped`): the box-day is incomplete and the owner is excluded for the run. A box whose ledger failed to open has no publisher and files nothing. The console fold asks each box's published day to prove the union of that box's durability, metrics and edge windows; a box with no ledger directory, no day file, or a ledger whose coverage starts after the box began serving holds the day for everyone. There is no tolerance. Durability days are counted per account: a box-day attested with an exclusion set is complete for every account the box did not exclude. The move worker's checkpoint is recorded against the route's owner. Nothing attests: attestation is false everywhere, in this code and in production. The scoped attestation (path A) was built, reviewed in round six, found unable to prove an exclusive writer, and removed from this branch. It is kept on `feat/carloku-scoped-attestation-20261001` with its findings open. Replication-write completeness is a launch blocker, not a waived requirement. |
| 4. `internal/awsbilling`: Cost and Usage Report reader (versioned deliveries only), newest-delivery selection, digests, charged S3 transfer classifier. It holds on a charged usage line it does not count and on any non-usage line (fee, refund, credit, discount) with a cost on a transfer usage type. Every transfer usage type the flagship account wrote from July to September 2026 is pinned in a test. |
| -5. `internal/objstore/ownership.go`: append-only bucket ownership ledger, opened at grant, closed at teardown. It resolves an owner only for a ledger that claims completeness, and only for hours that begin at or after the instant it began vouching, so a store's partial first hour never resolves (a ledger that began vouching exactly on the hour answers for that hour). Completeness needs a grant block recorded as minting, nothing delivered to the app yet, a probe showing the bucket does not exist, and an inventory showing no principal's policy names the bucket (`GranteeProber`), and is dated when both probes had answered. The AWS provisioner cannot list principals yet, so on AWS no ledger claims completeness, new stores included, and store transfer holds. A minting grant is refused while another app holds the name by ledger interval or by store record, and for a bucket that already exists and it never held. Grant, teardown and both rotation verbs are walks. A walk that spends backend authority takes a lease (`walklock.go`) on the bucket name, or on a hash of it when the name is one this build would refuse, or on a hash of the app when the record names no bucket: an object taken by compare-and-swap, bound to the invocation, thirty minutes long, standing until it runs out if the walk failed or any of its requests needed a second attempt. The record is read again under the lease and must still be the same estate, not only the same name: teardown and grant compare the grant instant, the key, how many grants have gone as far as creating anything, whether a teardown had begun, whether it noted the bucket deleted and how many take-backs it has seen; rotation compares the bucket, the key and that no teardown has begun. Every grant that creates anything is counted on the record before it calls the provider. A failed walk's lease stands until it runs out, with one exception: the same app re-running the same kind of walk may take it at once. A declaration-only teardown and the app purge take no lease; they delete the store record only as the generation they read. The user belongs to the app and the lease to the name, so a late request of a failed teardown can still remove the policy of a store granted afterwards under another name (round eight, Opus N2, open). Every request of a walk, to S3, to IAM and to the deployment bucket that holds the records and ledgers, is signed with the instant its lease was taken (`internal/sigfence`), so AWS refuses a request a paused process sends after its lease could be taken over. The record and its scan sidecar are deleted as the generation that was read. The signing-time rule was checked read-only against S3 and IAM with a list and two gets (`TestFenceAgainstAWS`); no delete was sent. A declaration-only teardown takes no lease (there is no bucket name) and relies on the conditional delete alone. A teardown writes a block on its own record before it touches anything and decides by compare-and-swap on the ledger (for a bucket name this build would refuse there is no ledger, and it decides from the store records alone): if nobody else holds the bucket it marks the bucket as being destroyed; otherwise it leaves the bucket and its scanner, revokes its own credentials, and records that the scanner is owed. A grant by the same app over its own unfinished teardown takes the store back, runs in full, and voids the teardown's note that the bucket was deleted. A teardown that leaves a bucket to another account keeps its record and returns `ErrBucketRetained` (discussion 191). It can finish once that bucket has been destroyed, when someone runs it again; nothing runs it again by itself. For a bucket name this build would refuse there is no ledger: a store whose teardown has begun is not counted as a holder there, so the last of two sharers destroys the bucket. `ErrBucketRetained` has no member copy yet; it needs Paul's review. A randomized simulation (`walk_sim_test.go`) checks the invariants over sequential walks with provider and store faults, including failures after the effect, and walks frozen past their lease while another takes the name. It does not generate a second bucket name, rotations, stores with no ledger interval or disagreeing clocks; those have direct tests. |
| +5. `internal/objstore/ownership.go`: append-only bucket ownership ledger, opened at grant, closed at teardown. It resolves an owner only for a ledger that claims completeness, and only for hours that begin at or after the instant it began vouching, so a store's partial first hour never resolves (a ledger that began vouching exactly on the hour answers for that hour). Completeness needs a grant block recorded as minting, nothing delivered to the app yet, a probe showing the bucket does not exist, and an inventory showing no principal's policy names the bucket (`GranteeProber`), and is dated when both probes had answered. The AWS provisioner cannot list principals yet, so on AWS no ledger claims completeness, new stores included, and store transfer holds. A minting grant is refused while another app holds the name by ledger interval or by store record, and for a bucket that already exists and it never held. Grant, teardown and both rotation verbs are walks. A walk that spends backend authority takes a lease (`walklock.go`) on the bucket name, or on a hash of it when the name is one this build would refuse, or on a hash of the app when the record names no bucket: an object taken by compare-and-swap, bound to the invocation, thirty minutes long, standing until it runs out if the walk failed or any of its requests needed a second attempt. The record is read again under the lease and must still be the same estate, not only the same name: teardown and grant compare the grant instant, the key, how many grants have reached the provider, whether a teardown had begun, whether it noted the bucket deleted and how many take-backs it has seen; rotation compares the same estate and requires that no teardown has begun. Every grant is counted on the record before it calls the provider at all, the user and its policy included, so a healthy store granted again writes its record once. A record's bucket is the one it is provisioned on, else the one its grant block names: the arguments choose a bucket only for a store that has none, so a grant never moves a store to another name. A record whose grant block and provisioned block name different buckets (an earlier build could write one) is refused by grant and teardown before they touch anything (`ErrTwoBuckets`); a person decides. A failed walk's lease stands until it runs out, with one exception: the same app re-running the same kind of walk may take it at once. A declaration-only teardown and the app purge take no lease; they delete the store record only as the generation they read. The user belongs to the app and the lease to the name, so a late request of a failed teardown can still remove the policy of a store granted afterwards under another name (round eight, Opus N2, open). Every request of a walk, to S3, to IAM and to the deployment bucket that holds the records and ledgers, is signed with the instant its lease was taken (`internal/sigfence`), so AWS refuses a request a paused process sends after its lease could be taken over. The record and its scan sidecar are deleted as the generation that was read. The signing-time rule was checked read-only against S3 and IAM with a list and two gets (`TestFenceAgainstAWS`); no delete was sent. A declaration-only teardown takes no lease (there is no bucket name) and relies on the conditional delete alone. A teardown writes a block on its own record before it touches anything and decides by compare-and-swap on the ledger (for a bucket name this build would refuse there is no ledger, and it decides from the store records alone): if nobody else holds the bucket it marks the bucket as being destroyed; otherwise it leaves the bucket and its scanner, revokes its own credentials, and records that the scanner is owed. A grant by the same app over its own unfinished teardown takes the store back, runs in full, and voids the teardown's note that the bucket was deleted. A teardown that leaves a bucket to another account keeps its record and returns `ErrBucketRetained` (discussion 191). It can finish once that bucket has been destroyed, when someone runs it again; nothing runs it again by itself. For a bucket name this build would refuse there is no ledger: a store whose teardown has begun is not counted as a holder there, so the last of two sharers destroys the bucket. `ErrBucketRetained` has no member copy yet; it needs Paul's review. A randomized simulation (`walk_sim_test.go`) checks the invariants over sequential walks with provider and store faults, including failures after the effect, and walks frozen past their lease while another takes the name. It does not generate a second bucket name, rotations, stores with no ledger interval or disagreeing clocks; those have direct tests. |
| 6. Statement folds: `console/durabilityfold.go` (`durability_writes_1k`, policy in `usage.BillableReplicationWrites`) and `console/storeegressfold.go` (`store_egress_gb`). `usage.MeterSource` names a source for both meters. Month evidence pins ownership ledgers by their month projection and knows the durability box directory. Statements carry the five billed meters by UTC day (`Statement.Days`), summing to the monthly totals; the other meters are not recorded by day. With a transfer report configured, the statements catch-up walks a 36-month window and sweeps every month that has an open statement, is missing a statement an account is owed (the platform's included), or is a gap between metered months. An account's statement that will not parse, or that cannot be read three passes running, is reported once per process and the month is classified without it. The platform's own statement is different: while it cannot be read the month stays unknown, for as long as that lasts. A month seen with no statement is remembered for six hours, a closed statement for the life of the process. |
| 7. Operator API doors (`billingoperator.go`): list and resolve billing review holds, list and set the house marker. No CLI verb yet. |
| -8. Team quantity worker (`billingquantity.go`) behind two switches, both unset: `CONSOLE_BILLING_PRORATION` (`immediate` or `next_invoice`, Paul's decision) and `CONSOLE_BILLING_QUANTITY_FENCE=stripe-sandbox-verified` (set only after `TestSandboxQuantityFence` has passed against a Stripe test-mode account; it has not been run). With either unset it makes no provider call. An operation is recorded by a compare-and-set on the billing row's revision, subscription, period, quantity and status, on the target and newest pending addition recomputed from the database, and on its own instant lying inside the period and not before the addition it charges for. The request dates its proration at that instant (`proration_date`). Sending and retiring exclude each other through a send lease on the operation's row, owned by one sender's token; an operation dated ahead of the observing clock is left alone. Every key ends in the account's operation serial. An operation nobody can show ran is acknowledged only after its idempotency key has been tried at the provider with a metadata-only request (the fence). If the fence request took the key, the operation never ran and may be retired, on a provider read taken after the fence when the subscription is current. If the key is held by the operation's own request, that request began and its outcome is not known: the operation is recorded as landed if the provider shows its target, and otherwise held with a review (`billing_quantity_operation_unsettled`) and never replaced until a person resolves it. A fence the provider refuses is held the same way. The fence rests on Stripe's documented key rules, is modelled by a fake in the tests, is not verified against Stripe, and lasts only as long as Stripe remembers a key (documented as at least 24 hours): `docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md`. A quantity that goes down inside a period files `billing_quantity_lowered`. An operation is confirmed only by the provider's success answer to its own request. A confirmed landing takes effect at the operation's instant, so a seat's charge and its allowance start together; a quantity that merely matches an unconfirmed operation's target is recorded as `landed_unverified`, dated when observed, with a review. If the provider refuses a request and names `proration_date`, the operation's key is fenced at once and it is retired on the next read; if it does not name it, the operation waits out the 22-hour replay limit. None of the provider's side is confirmed: test mode is not authorized. Only additions of the subscription's current period can raise it; older ones are closed uncharged with a review; additions and operations of an ended subscription are closed. It does not yet lower the quantity at renewal. |
| +8. Team quantity worker (`billingquantity.go`) behind two switches, both unset: `CONSOLE_BILLING_PRORATION` (`immediate` or `next_invoice`, Paul's decision) and `CONSOLE_BILLING_QUANTITY_FENCE=stripe-sandbox-verified` (set only after `TestSandboxQuantityFence` has passed against a Stripe test-mode account; it has not been run). With either unset it makes no provider call. An operation is recorded by a compare-and-set on the billing row's revision, subscription, period, quantity and status, on the target and newest pending addition recomputed from the database, and on its own instant lying inside the period and not before the addition it charges for. The request dates its proration at that instant (`proration_date`). Sending and retiring exclude each other through a send lease on the operation's row, owned by one sender's token; an operation dated ahead of the observing clock is left alone. Every key ends in the account's operation serial. An operation nobody can show ran is acknowledged only after its idempotency key has been tried at the provider with a metadata-only request (the fence), or after a person has resolved the review it was held with: a resolved review stands in for the fence, on that person's word and with no provider check. If the fence request took the key, the operation never ran and may be retired, on a provider read taken after the fence when the subscription is current. If the key is held by the operation's own request, that request began and its outcome is not known: the operation is recorded as landed if the provider shows its target, and otherwise held with a review (`billing_quantity_operation_unsettled`) and never replaced until a person resolves it. A fence the provider refuses is held the same way. The fence rests on Stripe's documented key rules, is modelled by a fake in the tests, is not verified against Stripe, and lasts only as long as Stripe remembers a key (documented as at least 24 hours): `docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md`. A quantity that goes down inside a period files `billing_quantity_lowered`. An operation is confirmed only by the provider's success answer to its own request. A confirmed landing takes effect at the operation's instant, so a seat's charge and its allowance start together; a quantity that merely matches an unconfirmed operation's target is recorded as `landed_unverified`, dated when observed, with a review. If the provider refuses a request and names `proration_date`, the operation's key is fenced at once and it is retired on the next read; if it does not name it, the operation waits out the 22-hour replay limit. None of the provider's side is confirmed: test mode is not authorized. Only additions of the subscription's current period can raise it; older ones are closed uncharged with a review; additions and operations of an ended subscription are closed. It does not yet lower the quantity at renewal. |
| 9. Stripe provisioning plan `stripe-provisioning.md` and print-only `stripe-provision.sh` (verified never to invoke the CLI). |
| 10. Partial-month billing: `plans.AllowanceForMonth` returns the paid spans of a month and the allowance weighted over them. A partly covered month (a first month, a month a subscription ends in, a lapse) bills only the UTC days wholly inside a paid span, against that allowance, and records the days it counted. Completeness is still whole-month. **The whole-paid-day rule is not in the approved spec, which speaks only of prorating the allowance. It is a proposal that needs Paul's approval; the plan document states what it rounds away and the tests that pin its boundary.** A month with a paid span and no whole paid day bills nothing. A month with no paid span is refused quietly. Two subscriptions in one month price and then hold on `billing_invoice_identity_mixed`. |
| 11. Plans: `docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md`, `2026-10-01-durability-attestation-path.md`, `2026-10-01-bucket-ownership-attestation.md`, `2026-10-01-partial-month-billing.md` (built). |
diff --git a/implementation/carloku-paid-launch/OUTSTANDING.md b/implementation/carloku-paid-launch/OUTSTANDING.md| index 879b4b7c4..25a4de612 100644 |
| --- a/implementation/carloku-paid-launch/OUTSTANDING.md |
| +++ b/implementation/carloku-paid-launch/OUTSTANDING.md |
| @@ -10,6 +10,14 @@ Three words are used exactly: |
| A passing test against a fake, a switch that is off, and an attestation that excludes the accounts it cannot vouch for are none of them "proven", and none is called complete below. |
| +Work is at one of these stages, and each item says which: |
| + |
| +- **Absent**: no code. |
| +- **Designed**: a written plan, no code. |
| +- **Partial, unwired**: code and tests on a side branch that nothing calls. Not gated, not reviewed. |
| +- **On the launch branch**: gated locally on the exact commit. No review round has passed and CI has not run. |
| +- **Proven**: as above. |
| + |
| ## Reviewable now |
| | Surface | Where | What it is | What it is not | |
| @@ -24,19 +32,19 @@ Each is concrete. None is waived. |
| ### Meters (the complete metered model cannot invoice until both are proven) |
| -1. **Replication writes (`durability_writes_1k`) cannot be proven on any box.** The external replicator writes outside the ledger. Path A (scoped attestation) was built, reviewed in round six, and cannot prove an exclusive writer; it is off this branch. Path B (the platform's own replicator as the only writer) is designed and not built, and needs a root unit change on every box under a scoped approval. Until then this meter is incomplete every month and nothing is invoiced. |
| +1. **Replication writes (`durability_writes_1k`) cannot be proven on any box.** The external replicator writes outside the ledger. Path A (scoped attestation) was built, reviewed in round six, and cannot prove an exclusive writer; it is off this branch and parked on `feat/carloku-scoped-attestation-20261001`. Path B (the platform's own replicator as the only writer) is **partial, unwired**, on `feat/carloku-metered-replicator-20261001` (`2140eb7b`, on the forge): a litestream store whose every replica client is metered with no unmetered fallback, an owner for each database the box replicates, and a fleet store that refuses anything it cannot meter, each with tests. **Absent**: the agent wiring that binds those meters to the box's ledger, a restore path that does not shell out to the external binary, the unit change, moving the replica credentials so nothing else on the box holds them (the unit runs as the same user as the agent today), and any attestation. Until all of that exists and is approved for a box, this meter is incomplete every month and nothing is invoiced. |
| 2. **Store transfer (`store_egress_gb`) has no source.** No Cost and Usage Report export exists on the account. It needs the scoped infrastructure approval, a first real delivery, an IAM principal inventory in the AWS provisioner (not built), and the bucket ownership attestation for stores that already exist (designed, not built). |
| -3. **A store's partial first hour** with a charged line holds the month, and nothing releases it. Needs a ruling: an operator release, or the platform absorbs it. |
| -4. **A box's first minutes** before its ledger opened hold that day for everyone. Needs a ruling. |
| +3. **A store's partial first hour** with a charged line holds the month, and nothing releases it. It holds by default and nobody is asked to waive it. What can release it is evidence: an operator door that records, for one bucket-hour, who is shown to have owned it and on what evidence, which the fold then honours. **Absent**; to be built locally. |
| +4. **A box's first minutes** before its ledger opened hold that day for everyone. Same as item 3: it holds by default, and the release is an operator evidence door, not a waiver. **Absent**. |
| ### Stripe |
| 5. **No catalog, portal configuration, webhook endpoint or runtime key exists.** The plan and a print-only script are written. Needs scoped approval, then a test-mode rehearsal, then a real purchase. |
| 6. **The quantity worker's fence is unverified against Stripe.** It rests on documented idempotency rules, modelled by a fake. `TestSandboxQuantityFence` must pass in a Stripe sandbox, under both proration policies, before `CONSOLE_BILLING_QUANTITY_FENCE` is set. Test mode is not authorized, so it has not been run. A pass is not enough by itself: check 5b logs what the worker's own update does with a declining card, and that log has to be read and decided on, because the worker records an addition as charged on any success and sends no `payment_behavior`. Two things it cannot settle and that stay open after it: what Stripe does with a key across an API key rotation, and how Stripe later reconciles a request it answered with a 500. |
| 6a. **An ended subscription's operation may need a person every time.** If Stripe refuses the fence request on a canceled subscription's item (the sandbox run logs what it answers), every canceled team subscription with an unconfirmed operation waits on a resolved review, and until then the account's quantity work is held, under a later subscription too. That needs a code decision from the sandbox log before the worker is switched on. |
| -7. **Defect, open: an old quantity request can run after Stripe has forgotten its key.** Stripe forgets keys after 24 hours. A sender frozen longer than that can still land a request for an operation that was retired and replaced. Today that is only detected after the write (`billing_quantity_lowered`), which is not a fix. It is being designed out on `feat/carloku-late-request-fencing-20261001` (`docs/superpowers/plans/2026-10-01-late-request-fencing.md`). The path that could cause it, the quantity worker, stays disabled until the design is built, reviewed and verified in a Stripe sandbox. Nobody is asked to accept this risk. |
| +7. **Defect, open: an old quantity request can run after Stripe has forgotten its key.** Stripe forgets keys after 24 hours. A sender frozen longer than that can still land a request for an operation that was retired and replaced. On the launch branch that is only detected after the write (`billing_quantity_lowered`), which is not a fix. The fix is **partial, unwired into this branch**, on `feat/carloku-late-request-fencing-20261001` (`68f77faf`, on the forge; design in `docs/superpowers/plans/2026-10-01-late-request-fencing.md`): a transport in which a request cannot outlive its connection, with tests. Its one premise, the provider's idle limit, is unmeasured. The path that could cause the defect, the quantity worker, stays disabled until that is gated, reviewed and measured in a Stripe sandbox. Nobody is asked to accept this risk. |
| 8. **Stripe's treatment of a proration date outside the period** is assumed from documentation. Same sandbox run. |
| -9. **Defect, open: a late invoice-line request can leave a finalized invoice with a wrong total.** (The fencing branch also voids such an invoice when it finds one. That is remediation after the fact: the invoice is payable until the void lands, and a customer who pays in that window is owed a refund. It does not close this item.) Past Stripe's key memory a stale line request runs as new. If it lands between the engine's line check and the finalize, the invoice is finalized with a duplicate line. The engine flags it and will not charge it, but a finalized invoice is open at Stripe, and an open invoice can be paid by the customer from Stripe's hosted page or the portal. That is customer-payable exposure and it is treated as such. It is being designed out on the same branch (a draft whose operations are not all confirmed inside one key-retention window is deleted, never finalized, and a deleted draft refuses every late request for good). Billing stays OFF, so the path is disabled. |
| +9. **Defect, open: a late invoice-line request can leave a finalized invoice with a wrong total.** Past Stripe's key memory a stale line request runs as new. If it lands between the engine's line check and the finalize, the invoice is finalized with a duplicate line. The engine flags it and will not charge it, but a finalized invoice is open at Stripe, and an open invoice can be paid by the customer from Stripe's hosted page or the portal. That is customer-payable exposure and it is treated as such. Prevention is the same transport as item 7, at the same stage (**partial, on the fencing branch**). That branch also voids such an invoice when it finds one; that is remediation after the fact, its limits are pinned by tests there (the invoice is payable until the void lands, and a customer who pays first is owed a refund), and it does not close this item. Billing stays OFF, so invoice writes are disabled until the prevention is proven. |
| ### Decisions that are Paul's |
| @@ -45,40 +53,41 @@ Each is concrete. None is waived. |
| 12. The whole-paid-day rule for partial months. The approved spec only says the allowance is prorated. The rule is tested at its boundaries and is a proposal. |
| 13. Member copy for the three ways a store walk can be refused: the bucket is retained by another account (`ErrBucketRetained`), held by another store (`ErrBucketHeld`), or busy with another walk (`ErrBucketBusy`). They reach the console as plain errors today. |
| -### Not built |
| +### Not built (required by the approved model) |
| -14. Quantity reduction at renewal. Team plans cannot be sold without it: a renewal bills the old quantity. |
| -15. Free tier: no Free entitlement, no app cap, no storage refusal, no per-plan idle, no flush-only class. |
| -16. Dormancy archive and warnings; recognised-revenue pool accounting; the books site. |
| +14. Quantity reduction at renewal. **Absent.** Team plans cannot be sold without it: a renewal bills the old quantity. |
| +15. Free tier. **Absent**: no Free entitlement, no app cap, no storage refusal, no per-plan idle, no flush-only class. |
| +16. Dormancy archive and warnings; recognised-revenue pool accounting; the books site. **Absent.** |
| 17. Per-line tax codes for a month spanning two tax codes (it holds with a review today). |
| 18. A `carlos` verb and documentation for the operator doors; a statement rebuild door for a closed month; an operator verb to backfill bucket ownership. |
| ### Process |
| -19. **No review round has passed.** Astra rejected rounds one to ten; Opus accepted each with fixes. The fixes to round ten's findings are unreviewed. |
| +19. **No review round has passed.** Astra rejected rounds one to eleven; Opus accepted each with fixes. Round eleven found one P1 and two P2 in the store walks (a grant could move an older record to another bucket name; a waiting rotation ignored a grant that ran; the user and its policy were written before the grant was counted). Its fixes are on the launch branch and unconfirmed. Each round's fixes are on the launch branch, gated locally, and unconfirmed until the next round reads them. |
| +19g. **Two store refusals tell the operator to run the grant again, which cannot work.** A record that names two buckets (`ErrTwoBuckets`) and a teardown note for a bucket that is not the record's (`ErrUnfinishedTeardown`) are refused before anything is touched, and the grant door answers "re-run the grant to resume". Neither state is written by this build; whether an earlier build left such records in production has not been checked. There is no operator verb to repair one, and correct copy needs approval (A). |
| 19f. **A store that leaves a bucket with an unsupported name does not hand on its scanner debt.** There is no ledger for such a name. The last holder would delete the bucket and leave a scanning plan behind. Today's scanner (ClamAV) has no plan to leave, so this is latent. |
| 19e. **A purge can end with a store that was granted during it.** A declare and a grant admitted while the app was live can land between the purge's last check and the removal of its marker. The purge now reports that as an error and never deletes the record, but it does not prevent it: the doors check liveness when they admit a request, not when they write. And after that error the estate is named only by its record and a log line. The app's console row is still trashed, so the settings page and the store-destroy door answer 404 and a second purge answers "not trashed", until the console restarts and restores the row from the record. The flash the member sees says "Nothing was deleted", which is false in this case; correct copy needs approval (A). |
| 19b. **Two round-eight findings are accepted and not fixed.** A late request of a failed teardown can remove the policy of a store the same app is granted afterwards under another name (the store goes dark; nothing is lost or leaked). A console whose clock is far ahead reserves a bucket name until that time, with no operator verb to clear it. |
| 19c. **S3's conditional delete is unverified on a real backend.** The store walks and the app purge delete a record only as the generation they read. That has run against the directory store and an HTTP stub. No `If-Match` delete has been sent to AWS or to an S3-compatible store, and a backend that ignored the header would delete unconditionally. It needs one write to a scratch object, which is a production-adjacent write nobody has approved. |
| 19d. **The signing-time fence has been observed on AWS only.** An S3-compatible deployment or store endpoint has not been probed for its window. |
| 19a. **A retained teardown finishes only when someone runs it again.** Nothing runs it. A store that left a bucket to another account keeps its record until then. |
| -20. **The branch tip is not on the forge.** From 03:30 on 2026-10-01 the hub's `carlos/platform` repository answered 500 or 502 after a ten-second wait, to reads and pushes alike, while the hub's front page and other repositories answered. Bundles are in the run directory. Amadan CI has not run on the tip. |
| +20. **Amadan CI has not run. External blocker.** The launch branch and the three side branches are on the forge. CI has been "queued" on every tip pushed tonight and has never started; the last check was on the tip named in `.claude/HANDOFF.md`, and an agent account can neither see nor manage the runners. The local gate (build, vet, gofmt, `make test`, cross builds) is not CI and is not offered in its place: a person has to get a runner to take the branch. The hub's `carlos/platform` repository also answered 500 on and off through the night; bundles of every tip are in the run directory. |
| 21. **A scanner backend that calls a provider must be fenced like the walks.** Today's scanner (ClamAV) makes no provider call at teardown. A native one would, and its client would need the fenced signer. |
| ## Approvals to ask for, each with its exact scope |
| -Nothing below was assumed overnight. None of it has been done. |
| +Nothing below was assumed overnight. None of it has been done. No row asks anyone to accept a defect: the open defects (items 7 and 9) and the uncertain intervals (items 3 and 4) stay held or disabled until they are fixed and proven. |
| | # | Approval | Exact scope | What it unblocks | |
| | --- | --- | --- | --- | |
| -| A | Copy | The two reviews as they stand at the URLs above (87 and 40 strings), the HALF code word, and member copy for the three store walk refusals | Applying copy byte for byte; nothing ships with unapproved copy | |
| +| A | Copy | The two reviews as they stand at the URLs above (87 and 40 strings), the HALF code word, and copy for the store refusals that have none or have the wrong one today: retained, held, busy, an unfinished teardown of another bucket, a record that names two buckets, and a store granted during a purge | Applying copy byte for byte; nothing ships with unapproved copy | |
| | B | Proration policy | One of `immediate` or `next_invoice` | Half of what the quantity worker needs. It does not start on this alone | |
| | C | Partial-month usage rule | Bill only UTC days wholly inside a paid span (`docs/superpowers/plans/2026-10-01-partial-month-billing.md`), or the alternative there | The first invoice of any subscriber who joins mid-month | |
| -| D | Two rulings | A store's partial first hour, and a box's first minutes before its ledger opened: operator release, or the platform absorbs them | Months that otherwise hold for good | |
| +| D | (none) | No ruling is asked for on a store's partial first hour or a box's first minutes. They hold by default. Releasing one takes an operator evidence door, which is engineering work (items 3 and 4), not an approval to waive | | |
| | E | Stripe test mode | One `rk_test_` restricted key with write on customers, payment methods, products, prices and subscriptions and read on invoices; permission to create and cancel test objects in the sandbox | Running `TestSandboxQuantityFence` under both policies (about ten minutes, plus a six-minute idle measurement on the fencing branch). Required before `CONSOLE_BILLING_QUANTITY_FENCE` is set | |
| | F | Stripe live provisioning | Products, prices, portal configuration, webhook endpoint and an `rk_live_` runtime key, exactly as `stripe-provisioning.md` lists them | A test purchase, then a real one. After A | |
| | G | AWS, read-write, scoped | Create the Cost and Usage Report data export in the collector plan. Separately: write, conditionally delete and remove one scratch object in the deployment bucket, to check that S3 honours `If-Match` on a delete (item 19c) | The store transfer meter's source; the conditional delete the store walks rely on | |
| -| H | Box change | Path B: replace the external replicator with the platform's own on a dev box, including which user holds the replica credentials | The replication meter. Needs building first | |
| +| H | Box change | Path B on a dev box: replace the external replicator with the platform's own, including which user holds the replica credentials. Not ready to ask for: the code is partial and unwired (item 1) | The replication meter | |
| | I | Merge | None requested. No review round has passed | | |
| ## Tested on this branch, not proven |
diff --git a/internal/objstore/objstore.go b/internal/objstore/objstore.go| index 8068a4059..2d6639507 100644 |
| --- a/internal/objstore/objstore.go |
| +++ b/internal/objstore/objstore.go |
| @@ -61,12 +61,13 @@ type GrantBlock struct { |
| // ownership ledger may claim completeness only for a minted bucket |
| // (ownership.go), so the absence of the field must never read as minted. |
| Minted bool `json:"minted,omitempty"` |
| - // Walks counts the grants that went past the point of creating |
| - // anything. It is written to the record before such a grant calls the |
| - // provider, so a grant that creates a bucket and delivers credentials |
| - // and then fails before it records them has still changed the record: |
| - // a walk that read the record earlier and waited for its lease can |
| - // tell that the store it was asked about is not the one standing now. |
| + // Walks counts the grants that reached the provider. It is written to |
| + // the record before a grant ensures the user, creates the bucket or |
| + // mints and delivers a key, so a grant that creates or restores |
| + // anything and then fails before it records the result has still |
| + // changed the record: a walk that read the record earlier and waited |
| + // for its lease can tell that the store it was asked about is not the |
| + // one standing now. |
| Walks int `json:"walks,omitempty"` |
| } |
diff --git a/internal/objstore/ownership.go b/internal/objstore/ownership.go| index 64a4ef6c6..fbf5aefbc 100644 |
| --- a/internal/objstore/ownership.go |
| +++ b/internal/objstore/ownership.go |
| @@ -78,6 +78,18 @@ type OwnerRef struct { |
| Since time.Time `json:"since"` |
| } |
| +// ErrUnfinishedTeardown: a grant was asked for one bucket while the store's |
| +// record carries an unfinished teardown of another. Running the grant again |
| +// will be refused again; the way forward is to finish the teardown. |
| +var ErrUnfinishedTeardown = errors.New("objstore: unfinished teardown of another bucket") |
| + |
| +// ErrTwoBuckets: the store's record names one bucket in its grant block and |
| +// another in its provisioned block. A walk would have to pick one and leave |
| +// the other behind a record that no longer leads to it, so the walk is |
| +// refused before it touches anything and a person decides (walk.go |
| +// oneBucket). |
| +var ErrTwoBuckets = errors.New("objstore: store record names two buckets") |
| + |
| // Refusals a walk gets from the ledger. Each is a state another walk put |
| // the bucket in, and each clears when that walk finishes. |
| var ( |
diff --git a/internal/objstore/ownership_test.go b/internal/objstore/ownership_test.go| index 094d62b47..acfccf55d 100644 |
| --- a/internal/objstore/ownership_test.go |
| +++ b/internal/objstore/ownership_test.go |
| @@ -7,6 +7,7 @@ import ( |
| "github.com/aws/aws-sdk-go-v2/aws" |
| "github.com/carlosframework/platform/internal/sigfence" |
| "net/http" |
| + "reflect" |
| "strings" |
| "testing" |
| "time" |
| @@ -2538,14 +2539,14 @@ func TestStaleTeardownDoesNotDestroyWhatAFailedGrantRetryMade(t *testing.T) { |
| } |
| } |
| -// A teardown's note that it deleted a bucket is evidence about that bucket. |
| -// Here an older record is provisioned on bucket X with no grant block, its |
| -// teardown deletes X and fails, and the app is then granted again. By the |
| -// grant's own rule its bucket is the minted name Y, which is another |
| -// bucket, with a ledger and holders of its own. The grant must not carry |
| -// X's deletion to Y's ledger, where it would tell a store that left Y that |
| -// its objects are gone. It is refused until X's teardown is finished. |
| -func TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother(t *testing.T) { |
| +// A record's bucket is the one it is provisioned on. Here an older record is |
| +// provisioned on bucket X with no grant block, its teardown deletes X and |
| +// fails, and the app is then granted again with no bucket named. The grant |
| +// is for X, the record's own bucket: not for the minted name Y, which is |
| +// another bucket with a ledger and holders of its own. So X's deletion is |
| +// recorded on X's ledger and nowhere else, and the store is never moved to |
| +// a name its record did not have. |
| +func TestGrantOverATeardownKeepsTheRecordsOwnBucket(t *testing.T) { |
| ctx := context.Background() |
| s := release.NewDirStore(t.TempDir()) |
| _, pub := testSealKeypair(t) |
| @@ -2580,9 +2581,24 @@ func TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother(t *testing.T) { |
| t.Fatalf("fixture: the record does not say which bucket was deleted: %+v", cur) |
| } |
| advance(walkLease + time.Minute) |
| - _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)) |
| - if err == nil { |
| - t.Fatal("a grant for Y went ahead over an unfinished teardown of X") |
| + // Whatever bucket the request names, minted or adopted. |
| + for _, adopt := range []string{"", ledgerBucket} { |
| + got, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, adopt, t0.Add(time.Hour)) |
| + if err != nil { |
| + t.Fatalf("adopt %q: the take-back of X: %v", adopt, err) |
| + } |
| + if got.Provisioned == nil || got.Provisioned.Bucket != legacyX || got.Grant == nil || got.Grant.Bucket != legacyX { |
| + t.Fatalf("adopt %q: the grant moved the store off its own bucket: %+v %+v", adopt, got.Provisioned, got.Grant) |
| + } |
| + if got.Grant.Minted { |
| + t.Fatalf("adopt %q: a bucket the record already had was written down as minted by this grant", adopt) |
| + } |
| + if got.Teardown != nil { |
| + t.Fatalf("adopt %q: the take-back left the teardown block: %+v", adopt, got.Teardown) |
| + } |
| + } |
| + if _, exists := p.Buckets[legacyX]; !exists { |
| + t.Fatal("the take-back did not bring X back") |
| } |
| ledger, _, lerr := ReadBucketOwnership(ctx, s, ledgerBucket) |
| if lerr != nil || ledger == nil { |
| @@ -2591,11 +2607,277 @@ func TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother(t *testing.T) { |
| if ledger.DestroyedAt != nil { |
| t.Fatalf("X's deletion was recorded on Y's ledger: %v", ledger.DestroyedAt) |
| } |
| - if cur, _, _ := Get(ctx, s, "bab", "titogo"); cur == nil || cur.Grant != nil || cur.Teardown == nil || cur.Teardown.BucketDeletedAt.IsZero() { |
| - t.Fatalf("the refused grant changed the record of the unfinished teardown: %+v", cur) |
| + if pol := p.Users[user].Policy; !strings.Contains(pol, legacyX) || strings.Contains(pol, ledgerBucket) { |
| + t.Fatalf("the user's policy reaches a bucket that is not the record's: %s", pol) |
| } |
| - // Finishing X's teardown is the way forward. |
| - if err := Teardown(ctx, s, p, nil, "bab", "titogo"); err != nil { |
| - t.Fatalf("finishing the teardown of X: %v", err) |
| +} |
| + |
| +// The same when the unfinished teardown never deleted anything: it left |
| +// bucket X to another account's store and reported it retained, with no |
| +// deletion note on the record. A grant with no bucket named is for X, where |
| +// this store's objects still are. It does not create the minted name Y and |
| +// clear the teardown block, which would leave X, and the other account's |
| +// reach into it, behind a record that no longer says so. |
| +func TestGrantOverARetainedBucketTakesThatBucketBack(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + advance := leaseClock(t) |
| + const legacyX = "carlos-store-legacy-x" |
| + p.Buckets[legacyX] = map[string][]byte{"mine.pdf": []byte("this store's object")} |
| + for _, o := range []struct{ acct, app string }{{"bab", "titogo"}, {"bac", "other"}} { |
| + user := UserName(o.acct, o.app) |
| + if _, err := p.EnsureUser(ctx, user, "/carlos/", "arn:aws:s3:::"+legacyX, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + rec := &Record{Declared: &Declared{EnvPrefix: "S3", At: t0, By: "member@example.com"}, |
| + Provisioned: &Provisioned{Bucket: legacyX, Region: "eu-west-1", IAMUser: user, KeyID: "K" + o.acct, At: t0}} |
| + if _, err := Put(ctx, s, o.acct, o.app, rec, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if err := Teardown(ctx, s, p, nil, "bab", "titogo"); !errors.Is(err, ErrBucketRetained) { |
| + t.Fatalf("fixture: the teardown that leaves X to another account: %v, want ErrBucketRetained", err) |
| + } |
| + if cur, _, _ := Get(ctx, s, "bab", "titogo"); cur == nil || cur.Teardown == nil || !cur.Teardown.BucketDeletedAt.IsZero() { |
| + t.Fatalf("fixture: want a teardown block with no deletion note: %+v", cur) |
| + } |
| + advance(time.Hour) |
| + got, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)) |
| + if err != nil { |
| + t.Fatalf("the take-back of X: %v", err) |
| + } |
| + if got.Provisioned == nil || got.Provisioned.Bucket != legacyX || got.Grant == nil || got.Grant.Bucket != legacyX || got.Teardown != nil { |
| + t.Fatalf("the grant did not take X back: %+v %+v %+v", got.Provisioned, got.Grant, got.Teardown) |
| + } |
| + if _, ok := p.Buckets[ledgerBucket]; ok { |
| + t.Fatal("the grant created the minted bucket") |
| + } |
| + if string(p.Buckets[legacyX]["mine.pdf"]) != "this store's object" { |
| + t.Fatal("the store's object in X is gone") |
| + } |
| +} |
| + |
| +// What is left of the refusal: a teardown block whose note names a bucket |
| +// that is not the record's own. No walk writes one (Teardown notes the |
| +// bucket it works from, which is the record's), so this is a record edited |
| +// by hand or by an older build. A grant does not guess: it would otherwise |
| +// carry the note to a ledger the note says nothing about. It is refused |
| +// with a sentinel that tells the caller that running it again will not |
| +// help, it makes no provider call, and it leaves the record alone. |
| +func TestGrantIsRefusedOverATeardownNoteForAnotherBucket(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + leaseClock(t) |
| + const elsewhere = "carlos-store-legacy-x" |
| + for _, withNote := range []bool{true, false} { |
| + td := &TeardownBlock{StartedAt: t0} |
| + if withNote { |
| + td.BucketDeletedAt, td.DeletedBucket = t0, elsewhere |
| + } else { |
| + td.DeletedBucket = elsewhere |
| + } |
| + rec := &Record{Declared: &Declared{EnvPrefix: "TITO_S3", At: t0, By: "member@example.com"}, Teardown: td} |
| + _, etag, _ := Get(ctx, s, "bab", "titogo") |
| + if _, err := Put(ctx, s, "bab", "titogo", rec, etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)) |
| + if !errors.Is(err, ErrUnfinishedTeardown) { |
| + t.Fatalf("note %v: a grant for the minted name over a teardown of %s: %v, want ErrUnfinishedTeardown", withNote, elsewhere, err) |
| + } |
| + cur, _, _ := Get(ctx, s, "bab", "titogo") |
| + if cur == nil || cur.Grant != nil || cur.Teardown == nil || cur.Teardown.DeletedBucket != elsewhere || cur.Teardown.TakeBacks != 0 { |
| + t.Fatalf("note %v: the refused grant changed the record: %+v", withNote, cur) |
| + } |
| + if len(p.Users) != 0 || len(p.Buckets) != 0 { |
| + t.Fatalf("note %v: the refused grant reached the provider: users %d buckets %d", withNote, len(p.Users), len(p.Buckets)) |
| + } |
| + if ledger, _, _ := ReadBucketOwnership(ctx, s, ledgerBucket); ledger != nil && ledger.DestroyedAt != nil { |
| + t.Fatalf("note %v: another bucket's deletion was recorded on the minted name's ledger", withNote) |
| + } |
| + // The refusal made no provider call: the name is not held for it. |
| + if w, werr := acquireBucketWalk(ctx, s, ledgerBucket, "bab", "second", "grant"); werr != nil { |
| + t.Fatalf("note %v: the refused grant left the name reserved: %v", withNote, werr) |
| + } else { |
| + w.release(true) |
| + } |
| + } |
| +} |
| + |
| +// A grant is counted on the record before it reaches the provider at all. |
| +// When that write fails, nothing was created: no user, no policy, no |
| +// bucket, no key. And a healthy store granted again is counted too, because |
| +// even that rewrites the user's policy. |
| +func TestGrantThatCannotBeCountedTouchesNothing(t *testing.T) { |
| + ctx := context.Background() |
| + dir := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + leaseClock(t) |
| + declare(t, dir, "bab", "titogo", "TITO_S3") |
| + // A first grant writes its grant block, then the count. |
| + failing := &failingPutStore{Store: dir, key: Key("bab", "titogo"), nth: 2} |
| + if _, err := Grant(ctx, failing, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err == nil || failing.seen < 2 { |
| + t.Fatalf("fixture: the grant was meant to fail at its count (err %v, writes %d)", err, failing.seen) |
| + } |
| + if len(p.Users) != 0 || len(p.Buckets) != 0 { |
| + t.Fatalf("a grant that was not counted reached the provider: users %+v, buckets %d", p.Users, len(p.Buckets)) |
| + } |
| + first, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // A healthy re-grant: the count is its first write. |
| + user := UserName("bab", "titogo") |
| + u := p.Users[user] |
| + u.Policy = "edited by hand" |
| + p.Users[user] = u |
| + failing = &failingPutStore{Store: dir, key: Key("bab", "titogo"), nth: 1} |
| + if _, err := Grant(ctx, failing, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); err == nil || failing.seen < 1 { |
| + t.Fatalf("fixture: the re-grant was meant to fail at its count (err %v, writes %d)", err, failing.seen) |
| + } |
| + if got := p.Users[user].Policy; got != "edited by hand" { |
| + t.Fatalf("a re-grant that was not counted rewrote the user's policy: %s", got) |
| + } |
| + again, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if again.Grant.Walks <= first.Grant.Walks { |
| + t.Fatalf("a healthy re-grant was not counted: %d then %d", first.Grant.Walks, again.Grant.Walks) |
| + } |
| + if again.Provisioned.KeyID != first.Provisioned.KeyID { |
| + t.Fatalf("a healthy re-grant minted a key: %s then %s", first.Provisioned.KeyID, again.Provisioned.KeyID) |
| + } |
| +} |
| + |
| +// A rotation reads its record and is held before its lease. A grant then |
| +// repairs the store: it mints and delivers a key and fails at its last |
| +// write, so the record still names the old key and the old grant instant. |
| +// The rotation wakes. It compares the whole estate, the count of grants |
| +// that reached the provider included, sees that a grant ran, and stops. It |
| +// does not reap the key the app was just given. |
| +func TestRotationStopsWhenAGrantRanWhileItWaited(t *testing.T) { |
| + for _, verb := range []string{"rotate", "finish"} { |
| + ctx := context.Background() |
| + dir := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + advance := leaseClock(t) |
| + declare(t, dir, "bab", "titogo", "TITO_S3") |
| + if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil { |
| + t.Fatal(err) |
| + } |
| + if verb == "finish" { |
| + if _, err := Rotate(ctx, dir, p, pub, "bab", "titogo", t0.Add(time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + user := UserName("bab", "titogo") |
| + var keysAfterGrant map[string]string |
| + s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)} |
| + s.hook = func() { |
| + // The deployment's endpoint has changed, so the grant walks the |
| + // store again and delivers a new key. Its count is its first |
| + // write of the record and the result its second, which fails. |
| + d := testDeployment() |
| + d.Endpoint = "https://s3.moved.example" |
| + before := len(p.Users[user].Keys) |
| + failing := &failingPutStore{Store: dir, key: Key("bab", "titogo"), nth: 2} |
| + _, err := Grant(ctx, failing, p, d, pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)) |
| + if err == nil || failing.seen < 2 { |
| + t.Errorf("%s: fixture: the grant was meant to fail at its last write (err %v, writes %d)", verb, err, failing.seen) |
| + } |
| + named := map[string]bool{} |
| + if cur, _, _ := Get(ctx, dir, "bab", "titogo"); cur != nil && cur.Provisioned != nil { |
| + named[cur.Provisioned.KeyID] = true |
| + named[cur.Provisioned.PrevKeyID] = true |
| + } |
| + unnamed := 0 |
| + for id := range p.Users[user].Keys { |
| + if !named[id] { |
| + unnamed++ |
| + } |
| + } |
| + if unnamed == 0 { |
| + t.Errorf("%s: fixture: the grant delivered no key the record does not name (keys before %d, now %d)", verb, before, len(p.Users[user].Keys)) |
| + } |
| + keysAfterGrant = map[string]string{} |
| + for id, secret := range p.Users[user].Keys { |
| + keysAfterGrant[id] = secret |
| + } |
| + advance(walkLease + time.Minute) |
| + } |
| + var err error |
| + if verb == "rotate" { |
| + _, err = Rotate(ctx, s, p, pub, "bab", "titogo", t0.Add(2*time.Hour)) |
| + } else { |
| + _, err = RotateFinish(ctx, s, p, "bab", "titogo") |
| + } |
| + if !s.done { |
| + t.Fatalf("%s: fixture: the rotation never reached its lease", verb) |
| + } |
| + if !errors.Is(err, release.ErrCASConflict) { |
| + t.Fatalf("%s after a grant ran while it waited: %v, want it to stop", verb, err) |
| + } |
| + if got := p.Users[user].Keys; !reflect.DeepEqual(got, keysAfterGrant) { |
| + t.Fatalf("%s: the stopped rotation changed the user's keys: %v, were %v", verb, got, keysAfterGrant) |
| + } |
| + } |
| +} |
| + |
| +// A record whose grant block names one bucket and whose provisioned block |
| +// names another: what an earlier build left when it granted the minted name |
| +// over an older record provisioned elsewhere. A teardown would destroy one |
| +// bucket, delete the record and leave the other with nothing naming it; a |
| +// grant would pick one. Both are refused before they lease, reach the |
| +// provider or write anything. |
| +func TestWalksRefuseARecordThatNamesTwoBuckets(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + leaseClock(t) |
| + const legacyX = "carlos-store-legacy-x" |
| + user := UserName("bab", "titogo") |
| + if _, err := p.EnsureUser(ctx, user, "/carlos/", "arn:aws:s3:::"+legacyX, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + p.Buckets[legacyX] = map[string][]byte{"old.pdf": []byte("in X")} |
| + p.Buckets[ledgerBucket] = map[string][]byte{"new.pdf": []byte("in Y")} |
| + rec := &Record{Declared: &Declared{EnvPrefix: "TITO_S3", At: t0, By: "member@example.com"}, |
| + Grant: &GrantBlock{Backend: "aws", Bucket: ledgerBucket, Source: GrantSourceOperator, At: t0, By: "op", Minted: true}, |
| + Provisioned: &Provisioned{Bucket: legacyX, Region: "eu-west-1", IAMUser: user, KeyID: "LEGACYKEY", At: t0}} |
| + if _, err := Put(ctx, s, "bab", "titogo", rec, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + before, _, _ := Get(ctx, s, "bab", "titogo") |
| + policy := p.Users[user].Policy |
| + if _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); !errors.Is(err, ErrTwoBuckets) { |
| + t.Fatalf("a grant over a record that names two buckets: %v, want ErrTwoBuckets", err) |
| + } |
| + if err := Teardown(ctx, s, p, nil, "bab", "titogo"); !errors.Is(err, ErrTwoBuckets) { |
| + t.Fatalf("a teardown of a record that names two buckets: %v, want ErrTwoBuckets", err) |
| + } |
| + after, _, _ := Get(ctx, s, "bab", "titogo") |
| + if !reflect.DeepEqual(before, after) { |
| + t.Fatalf("a refused walk changed the record:\n%+v\n%+v", before, after) |
| + } |
| + if string(p.Buckets[legacyX]["old.pdf"]) != "in X" || string(p.Buckets[ledgerBucket]["new.pdf"]) != "in Y" { |
| + t.Fatal("a refused walk destroyed objects") |
| + } |
| + if _, ok := p.Users[user]; !ok || p.Users[user].Policy != policy { |
| + t.Fatal("a refused walk changed the user or its policy") |
| + } |
| + for _, b := range []string{legacyX, ledgerBucket} { |
| + w, err := acquireBucketWalk(ctx, s, b, "bab", "second", "grant") |
| + if err != nil { |
| + t.Fatalf("a refused walk left %s reserved: %v", b, err) |
| + } |
| + w.release(true) |
| } |
| } |
diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go| index 5554e95db..08d0aa7d1 100644 |
| --- a/internal/objstore/walk.go |
| +++ b/internal/objstore/walk.go |
| @@ -128,8 +128,9 @@ var bucketNameRe = regexp.MustCompile(`^[a-z0-9][a-z0-9.-]{1,61}[a-z0-9]$`) |
| func ValidBucketName(name string) bool { return bucketNameRe.MatchString(name) } |
| // Grant provisions the declared store and delivers its credentials, in spec |
| -// §5 order. It is idempotent: a healthy provisioned store makes a re-run a |
| -// no-op (no fresh key, no rewrite), and an unhealthy one — a crash anywhere |
| +// §5 order. It is idempotent: a healthy provisioned store makes a re-run |
| +// mint no key and deliver nothing (it rewrites the user's policy and counts |
| +// itself on the record, GrantBlock.Walks), and an unhealthy one — a crash anywhere |
| // in the sequence — is repaired by walking it again. adoptBucket names an |
| // existing bucket to adopt instead of minting one; it is honoured only on the |
| // FIRST grant, since the recorded bucket is what the app's data is in. |
| @@ -155,6 +156,9 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| if rec == nil || rec.Declared == nil { |
| return nil, ErrNotDeclared |
| } |
| + if err := oneBucket(rec, acct, app); err != nil { |
| + return nil, err |
| + } |
| // A grant over an unfinished teardown takes the store back. The record |
| // says a teardown began (TeardownBlock), so whatever its Provisioned |
| // block and delivered variables say, the user or the bucket may be |
| @@ -173,15 +177,22 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| // Which bucket this grant is for, as a rule of the record and the |
| // arguments: applied here to choose the name to lease, and again to the |
| // record read under the lease. |
| + // A record that already has a bucket keeps it: the one it is |
| + // provisioned on, else the one its grant block names, in the order |
| + // Teardown reads them. The arguments choose a bucket only for a store |
| + // that has none. So a grant can never move a store to another name and |
| + // leave the old bucket, or a teardown of it, behind a record that |
| + // names something else. |
| bucketFor := func(r *Record) string { |
| - b := adoptBucket |
| - if r.Grant != nil && r.Grant.Bucket != "" { |
| - b = r.Grant.Bucket // a re-run never re-chooses the bucket |
| - } |
| - if b == "" { |
| - b = MintedBucket(d, acct, app) |
| + switch { |
| + case r.Provisioned != nil && r.Provisioned.Bucket != "": |
| + return r.Provisioned.Bucket |
| + case r.Grant != nil && r.Grant.Bucket != "": |
| + return r.Grant.Bucket // a re-run never re-chooses the bucket |
| + case adoptBucket != "": |
| + return adoptBucket |
| } |
| - return b |
| + return MintedBucket(d, acct, app) |
| } |
| bucket := bucketFor(rec) |
| // The estate this grant was asked about, as first read (Teardown keeps |
| @@ -228,29 +239,36 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| if rec.Provisioned != nil { |
| startKeyID = rec.Provisioned.KeyID |
| } |
| - // The bucket an unfinished teardown of this record deleted, if it noted |
| - // one. Read before this call writes a grant block of its own. |
| - deletedBucket := "" |
| - if rec.Teardown != nil && !rec.Teardown.BucketDeletedAt.IsZero() { |
| + // The bucket an unfinished teardown of this record was taking apart: the |
| + // one its note names, else the record's own coordinates, which are what |
| + // Teardown works from. Read before this call writes a grant block. |
| + tornBucket, deletedBucket := "", "" |
| + if rec.Teardown != nil { |
| switch { |
| case rec.Teardown.DeletedBucket != "": |
| - deletedBucket = rec.Teardown.DeletedBucket |
| + tornBucket = rec.Teardown.DeletedBucket |
| case rec.Provisioned != nil: |
| - deletedBucket = rec.Provisioned.Bucket |
| + tornBucket = rec.Provisioned.Bucket |
| case rec.Grant != nil: |
| - deletedBucket = rec.Grant.Bucket |
| + tornBucket = rec.Grant.Bucket |
| + } |
| + // A take-back is for the bucket the teardown was taking apart, or |
| + // it is refused. A grant for another name would clear the teardown |
| + // block and leave the old bucket behind a record that no longer |
| + // names it: left to another account with nothing saying so, or |
| + // half destroyed with its ledger stuck. And it would carry any note |
| + // that the old bucket was deleted to a ledger the note says nothing |
| + // about. The teardown is finished first. |
| + if tornBucket != "" && tornBucket != bucket { |
| + return nil, fmt.Errorf("%w: %s/%s has an unfinished teardown of bucket %s; finish it before granting %s", ErrUnfinishedTeardown, acct, app, tornBucket, bucket) |
| } |
| - // The note is about that bucket. A take-back for another name |
| - // would carry it to a ledger it says nothing about, and leave the |
| - // old name's teardown unfinished behind a record that no longer |
| - // names it. The teardown is finished first. |
| - if deletedBucket != bucket { |
| - return nil, fmt.Errorf("objstore: %s/%s has an unfinished teardown of bucket %s; finish it before granting %s: %w", acct, app, deletedBucket, bucket, release.ErrCASConflict) |
| + if !rec.Teardown.BucketDeletedAt.IsZero() { |
| + deletedBucket = tornBucket |
| } |
| } |
| wroteGrant := rec.Grant == nil |
| if wroteGrant { |
| - rec.Grant = &GrantBlock{Backend: d.Backend, Bucket: bucket, Source: source, At: now, By: actor, Minted: adoptBucket == ""} |
| + rec.Grant = &GrantBlock{Backend: d.Backend, Bucket: bucket, Source: source, At: now, By: actor, Minted: adoptBucket == "" && rec.Provisioned == nil} |
| } |
| // A grant over an unfinished teardown voids what that teardown noted |
| // about the bucket, before this walk does anything that could bring |
| @@ -392,6 +410,32 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| } |
| return nil, err |
| } |
| + // From here the walk changes things at the provider: the user and its |
| + // policy first, then the bucket and the keys. It is counted on the |
| + // record before any of that (GrantBlock.Walks), a healthy re-grant |
| + // included, because even that rewrites the user's policy. If this grant |
| + // creates or restores anything and then fails before its last write, |
| + // the record still differs from what any waiting walk read before it |
| + // began. |
| + // |
| + // The console's declare door writes this same object. If it wrote in |
| + // between, the count is put on the record as it now stands, provided |
| + // it is still the same estate; anything else is a conflict. |
| + before := estateOf(rec) |
| + rec.Grant.Walks++ |
| + if etag, err = Put(ctx, s, acct, app, rec, etag); errors.Is(err, release.ErrCASConflict) { |
| + fresh, freshEtag, gerr := Get(ctx, s, acct, app) |
| + if gerr != nil || fresh == nil || fresh.Declared == nil || fresh.Grant == nil || estateOf(fresh) != before { |
| + return nil, err |
| + } |
| + fresh.Grant.Walks++ |
| + if etag, err = Put(ctx, s, acct, app, fresh, freshEtag); err != nil { |
| + return nil, err |
| + } |
| + rec = fresh |
| + } else if err != nil { |
| + return nil, err |
| + } |
| // 2. the principal and its policy. EnsureUser runs UNCONDITIONALLY, before |
| // the idempotence short-circuit below — the same fix internal/mail's walk |
| // made as "C2", for the same reason. |
| @@ -434,28 +478,6 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| return rec, nil |
| } |
| } |
| - // From here the walk creates things. It is counted on the record first |
| - // (GrantBlock.Walks): if this grant makes a bucket and delivers |
| - // credentials and then fails before its last write, the record must |
| - // still differ from what any waiting walk read before it began. |
| - // The console's declare door writes this same object. If it wrote in |
| - // between, the count is put on the record as it now stands, provided |
| - // it is still the same estate; anything else is a conflict. |
| - before := estateOf(rec) |
| - rec.Grant.Walks++ |
| - if etag, err = Put(ctx, s, acct, app, rec, etag); errors.Is(err, release.ErrCASConflict) { |
| - fresh, freshEtag, gerr := Get(ctx, s, acct, app) |
| - if gerr != nil || fresh == nil || fresh.Declared == nil || fresh.Grant == nil || estateOf(fresh) != before { |
| - return nil, err |
| - } |
| - fresh.Grant.Walks++ |
| - if etag, err = Put(ctx, s, acct, app, fresh, freshEtag); err != nil { |
| - return nil, err |
| - } |
| - rec = fresh |
| - } else if err != nil { |
| - return nil, err |
| - } |
| // 3. the bucket |
| region, err := p.EnsureBucket(ctx, bucket) |
| if err != nil { |
| @@ -701,7 +723,13 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app |
| // finished in between, and the keys this rotation would touch belong to |
| // whatever stands there now. |
| func leaseForRotation(ctx context.Context, s release.Store, acct, app string, first *Record) (context.Context, *Record, string, func(error), error) { |
| - bucket, keyID := first.Provisioned.Bucket, first.Provisioned.KeyID |
| + bucket := first.Provisioned.Bucket |
| + // The estate the rotation was asked about, as Teardown and Grant keep |
| + // it. It includes the count of grants that reached the provider: a |
| + // grant that repaired the store while this rotation waited may have |
| + // delivered a key the record does not name yet, and a rotation that |
| + // went ahead would reap it. |
| + asked := estateOf(first) |
| walk, err := acquireBucketWalk(ctx, s, bucket, acct, app, "rotate") |
| if err != nil { |
| return ctx, nil, "", nil, err |
| @@ -716,7 +744,7 @@ func leaseForRotation(ctx context.Context, s release.Store, acct, app string, fi |
| case err != nil: |
| case rec == nil || rec.Provisioned == nil: |
| err = ErrNotProvisioned |
| - case rec.Provisioned.Bucket != bucket || rec.Provisioned.KeyID != keyID || rec.Teardown != nil: |
| + case rec.Teardown != nil || estateOf(rec) != asked: |
| err = fmt.Errorf("objstore: store record for %s/%s changed while its rotation was starting; run it again: %w", acct, app, release.ErrCASConflict) |
| } |
| if err != nil { |
| @@ -761,6 +789,13 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a |
| if rec == nil { |
| return nil |
| } |
| + // The record is deleted last because it is the only thing naming the |
| + // estate. A record that names two buckets cannot be followed to both: |
| + // this walk would destroy one, delete the record, and leave the other |
| + // with nothing naming it. |
| + if err := oneBucket(rec, acct, app); err != nil { |
| + return err |
| + } |
| // What this walk believes it is destroying — the guard on the final delete. |
| startKeyID, startBucket := "", "" |
| if rec.Provisioned != nil { |
| @@ -1183,7 +1218,8 @@ func estateOf(rec *Record) storeEstate { |
| // out here and can still run there, so the lease is not freed. |
| func walkEndedClean(err error) bool { |
| return err == nil || errors.Is(err, ErrBucketRetained) || errors.Is(err, ErrBucketHeld) || |
| - errors.Is(err, ErrBucketBeingDestroyed) || errors.Is(err, ErrNotDeclared) || errors.Is(err, ErrNoPendingFinish) |
| + errors.Is(err, ErrBucketBeingDestroyed) || errors.Is(err, ErrNotDeclared) || errors.Is(err, ErrNoPendingFinish) || |
| + errors.Is(err, ErrUnfinishedTeardown) |
| } |
| // grantElapsed is how long a grant has been running. A seam, so a test can |
| @@ -1371,3 +1407,17 @@ func writeEnv(ctx context.Context, s release.Store, sealPub *ecdh.PublicKey, |
| } |
| return nil |
| } |
| + |
| +// oneBucket refuses a record whose grant block and provisioned block name |
| +// different buckets. No walk in this build writes one: a grant's bucket is |
| +// the record's provisioned bucket when it has one (bucketFor in Grant). An |
| +// earlier build could, by granting the minted name over an older record |
| +// that was provisioned elsewhere and had no grant block. Which of the two |
| +// holds the store's objects, and which the user's policy reaches, cannot be |
| +// told from the record, so neither Grant nor Teardown guesses. |
| +func oneBucket(rec *Record, acct, app string) error { |
| + if rec.Grant == nil || rec.Provisioned == nil || rec.Grant.Bucket == "" || rec.Provisioned.Bucket == "" || rec.Grant.Bucket == rec.Provisioned.Bucket { |
| + return nil |
| + } |
| + return fmt.Errorf("%w: %s/%s is provisioned on %s and its grant names %s", ErrTwoBuckets, acct, app, rec.Provisioned.Bucket, rec.Grant.Bucket) |
| +} |