carlos / platform Public

Close round eight: an estate is not a name, every walk is leased, a purge never sweeps a granted store, a review names its operation

Round eight was narrower again (dispositions, "Round eight"). Fixed:

- A teardown keeps the estate it was asked about (grant instant, key) and
  stops if the record under the lease is another estate. The minted name is
  the same for every store an app ever has.
- Every walk that spends backend authority takes a lease. A bucket name this
  build would refuse is leased under a hash of itself, a record with no bucket
  under a hash of its app. Failing validation is never a reason to run
  unleased.
- The app purge deletes a store declaration only as the generation its guard
  read, and never in the unconditional sweep. A grant that lands during the
  purge keeps its record.
- The unsettled review names the operation by serial, so one operation's
  resolution cannot settle another created in the same second.
- A fence request that ends in a remembered server error files the review.
- The production constructors are tested for the signer and the attempt
  watcher: the deployment bucket's store, the provisioner's S3 and IAM clients.
- Simulations: pending additions are excused only by the blocking operation's
  own review; a person resolves reviews; a re-grant of a live store is checked
  for its data; a walk is held before its lease while the store is replaced.
- The sandbox procedure gains a declining card, a remembered 4xx that must
  carry the replay mark, and a stricter canceled-subscription check. Not run.

Accepted and not fixed, recorded in OUTSTANDING.md: the user belongs to the
app while the lease belongs to the name; a far-ahead clock reserves a name
with no operator verb to clear it; S3's conditional delete and an
S3-compatible store's signing window have not been checked on a real backend.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude 3 weeks ago 14d3af8907a9a4b27fc5c83c19b1f99a7e76a40d parent 5712679

17 files changed, +1316 −85

docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md +39 −0
docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round8.md +149 −0
docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round8.md +249 −0
implementation/carloku-paid-launch/AGENT_HANDOFF.md +3 −3
implementation/carloku-paid-launch/OUTSTANDING.md +6 −3
internal/billingstripe/sandbox_fence_test.go +79 −12
internal/console/billingquantity.go +28 −11
internal/console/billingquantity_sim_test.go +65 −7
internal/console/billingquantity_test.go +135 −3
internal/objstore/fence_aws_test.go +12 −9
internal/objstore/fence_wiring_test.go +106 −0
internal/objstore/ownership_test.go +128 −0
internal/objstore/walk.go +39 −11
internal/objstore/walk_sim_test.go +101 −10
internal/objstore/walklock.go +22 −1
internal/release/trash.go +61 −15
internal/release/trash_test.go +94 −0

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 f1c4bc624..563628763 100644

--- a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md

+++ b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md

@@ -283,3 +283,42 @@ Each removes one fix. Each fails the named test, except the last.

- Storage: the backend never refuses a stale walk (`TestPausedWalkCannotActAfterItsLeaseIsTaken` and the simulation); the record delete unconditional (`TestDeclarationOnlyTeardown…`); teardown not comparing its record with its lease; grant not recomputing its bucket; a doubtful walk freeing the name; a store that left counted as holding (its test, and the simulation at seed 1360); rotation ignoring a replaced store; rotation leasing another name; destruction not recorded at the deletion; the lease margin shortened; the sidecar delete unconditional.

- Quantity: a key held by its own request made retirable; the send lease taken while live; any sender releasing it; the key without its serial; a replayed 500 replayed again (its test, and the simulation at seed 4); an unresolved review letting the operation be retired.

- Equivalent: removing the "review is open, do not ask the provider again" branch. The branch after it returns without asking as well.

+## Round eight (reviewed `c3e87a94`; Astra rejected, Opus accepted with fixes)

+Reports: `CODEX-FINDINGS-carloku-paid-launch-overnight-round8.md`, `OPUS-REVIEW-carloku-paid-launch-overnight-round8.md`.

+Fourteen findings in round six, nine in seven, five from Astra in eight. Neither reviewer found a way to attest, to reach Stripe with a switch unset, or to make either open late-request defect reachable. What they found is narrower each round and is the same kind of thing: something a walk or the worker acts on that the lease or the key does not identify.

+| --- | --- | --- | --- |

+| Astra F1: the app purge sweeps `store.json` unconditionally, over a grant that landed after its guard | Accepted. The purge deletes the store declaration only as the generation its guard read, first, and applies the same rule if the key appears in the sweep and once more before the marker goes. A record that carries estate, or that changed, refuses the purge. | `release/trash.go` `purgeStoreDeclaration` | `TestPurgeAppNeverSweepsAStoreRecordGrantedDuringThePurge` (four landing points) |

+| Astra F2, Opus N6: a record with an invalid bucket name, or none, is walked with no lease and no fence | Accepted. Every walk that spends backend authority takes a lease. A name that cannot be an object key is leased under a hash of itself, and a record with no bucket under a hash of its app. | `walklock.go` `walkLeaseKey`; `walk.go` | `TestStoreWithAnUnsupportedBucketNameIsStillLeased` |

+| Opus N1: a stale teardown destroys a store re-created under the same minted name | Accepted. The teardown keeps the estate it was asked about (grant instant, key, whether a teardown had begun) and stops if the record under the lease is another estate, name or no name. | `walk.go` `estateOf` | `TestStaleTeardownDoesNotDestroyAStoreRecreatedUnderTheSameName` |

+| Astra F3, Opus N4: a resolved review is keyed by the operation's creation second and can settle another operation | Accepted. The review names the operation's serial. | `billingquantity.go` `billingQuantityUnsettledReason` | `TestQuantityResolvedReviewSettlesOnlyItsOwnOperation` |

+| Astra F4: a fence request that itself ends in a remembered 500 files no review | Accepted. A server error answered from memory on the fence request files the operation's review and is not asked again while it is open. | `fenceQuantityOperation` | `TestQuantityFenceThatEndsInARememberedServerErrorIsHeldForAPerson` |

+| Astra F5, Opus N7: the quantity simulation excuses pending additions with any open review | Accepted. Only the outstanding operation's own review, or a mismatch review when nothing is outstanding, counts. The simulation now also has a person resolve reviews. | `billingquantity_sim_test.go` `quantityHeldBy` | `TestQuantitySimulationOracleIgnoresUnrelatedReviews` |

+| Opus N5: nothing tests that the production constructors carry the signer and the watcher | Accepted. | `objstore/fence_wiring_test.go` | `TestProductionClientsCarryTheFenceAndTheAttemptWatcher` (the store, the provisioner's S3 client and its IAM client; fails if either line is removed from any of them) |

+| Astra, Opus N7: storage simulation oracle | Accepted in part. A second grant of a live store is checked for its data before the model writes it again (with a test of that check). A walk is held between its first read and its lease while the store is torn down and granted again. A walk refused before it began, or fenced on waking, no longer overwrites what another walk of the same app left. Not done: ownership is still sampled, not checked mid-walk; a user's existence still stands in for working credentials. | `walk_sim_test.go` | `TestSimulationOracleSeesARegrantThatLosesData` |

+| Astra, Opus N8, N12: the sandbox procedure | Accepted. A declining card under the immediate policy, with a remembered 402 that must carry the replay mark (the nearest reproducible stand-in for a remembered 500); the canceled check requires a refusal of the request and reads the quantity again; a fence re-sent after losing a race must read as held by the request; goroutines no longer fail the test from outside its goroutine; the cleanup comment says what is left behind. | `sandbox_fence_test.go` | not run |

+| Astra, AWS probe: the retry part logs and does not assert; the escaped-key check accepts too much | Accepted. The comment now says the probe does not establish retry behaviour. The escaped key must be answered `NoSuchKey` or `AccessDenied`. | `fence_aws_test.go` | none |

+### Accepted and not fixed

+- **Opus N2: the user belongs to the app, the lease to the bucket name.** A failed teardown of an adopted bucket whose `DeleteUserPolicy` is still in transit, the same app's re-run finishing at once, and a re-grant on a different name can leave a store whose user has lost its policy while its record reads healthy. No data is lost and nothing leaks; the store is dark until it is granted again. The fix is a second lease keyed on the app, carried the same way. Not built.

+- **Opus N3: a console whose clock is a day ahead reserves a name for a day**, and a same-app retry inherits the reservation. A hold, not a safety failure. The fix is to treat a lease further ahead than its own length as corrupt and give an operator a verb to clear it. Not built.

+- **Opus N7: the dating review in the quantity simulation's Held check** is still matched by account, not by operation. Events do not carry their operation's identity.

+- **The simulation cannot see Opus N1.** It now generates the schedule, but its model does not tell a request made against one store from a request made against its successor, so a stale teardown that destroys the new store reads as an ordinary teardown. The direct test is what covers it.

+### Claims corrected

+- The lease and the fence apply to every walk that spends backend authority, with no exception for invalid names. A declaration-only teardown and the app purge take no lease and rely on conditional deletes.

+- "The backend refuses the delete": S3's conditional delete has been exercised against the directory store and an HTTP stub only. Nothing has sent `If-Match` on a delete to AWS or to an S3-compatible store. A backend that ignores the header deletes unconditionally. The fence's signing window is the other layer, and it has been observed on AWS only.

+- "A resolved review belongs to that very operation": now true, by serial.

+- "Every unconfirmed acknowledgement first consumes the key at the provider": not when a person's resolved review stands in for the fence. That exception is stated where the rule is.

+- "A retirement follows a provider read taken after the fence": for an operation of the current subscription. An ended subscription's operation, and one whose subscription changed, are acknowledged without a read.

+- The worker's hold on a replayed server error reads Stripe's `Idempotent-Replayed` header on an error answer. That it is present on errors is not documented in what was quoted; the sandbox run checks it on a remembered 402. If it is absent the hold never fires and such an operation waits out the 22-hour limit, which is safe.

+### Mutation checks (scratch copy)

+Each fails its test: the estate comparison removed; an unsupported name walked with no lease; the production store without its signer; the provisioner without its attempt watcher; the purge sweeping the record unconditionally; the purge deleting the declaration unconditionally; the review keyed by creation second; a replayed fence error filing nothing.

diff --git a/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round8.md b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round8.md

new file mode 100644

index 000000000..c0fa55fe6

--- /dev/null

+++ b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round8.md

@@ -0,0 +1,149 @@

+## 1. Round-seven dispositions

+Reviewed `c3e87a94`, base `aed42815`, with particular attention to `43383b93..c3e87a94`. This was a static review: no files changed, tests run, or network commands executed. Counterfactual test assessments come from reading implementations and assertions.

+All references name files at `c3e87a94`. Abbreviations: **O** = `internal/objstore`, **C** = `internal/console`, **B** = `internal/billingstripe`, **S** = `internal/sigfence`, **R** = `internal/release`.

+|---|---|---|---|

+| Astra F1 | **Closed for the reported declaration-teardown race.** Another declaration-only deletion path remains; new F1 below. | Conditional deletion: `O/walk.go:1169`. Test: `O/ownership_test.go:1666`. | **No.** The test requires the conditional-delete hook and preservation of the newly granted record. |

+| Astra F3 | **Closed for the deployment-store signer omission.** Constructor wiring lacks a direct regression. | `R/store.go:555`; `S/sigfence.go:79`; conditional sidecar deletion at `O/walk.go:1188`. Tests: `S/sigfence_test.go:19`, `O/ownership_test.go:2067`. | **Yes** if only `NewS3Store`’s signer wiring is removed: these fixtures bypass that constructor. **No** if the sidecar’s conditional deletion is removed. |

+| Astra F4 | **Closed for successful SDK retries losing uncertainty.** | `S/sigfence.go:123`; `O/walklock.go:200`. Tests: `S/sigfence_test.go:53`, `O/ownership_test.go:1761`. | **No** for the corresponding watcher or reservation mutation. The SDK test observes two attempts; the walk test injects doubt explicitly. |

+| Astra F5 | **Closed for retiring an indeterminate original request solely on a mismatch.** | Separate retirement predicate: `C/billingquantity.go:706`, `:1221`, `:1236`. Test: `C/billingquantity_test.go:2089`. Replayed-update-500 handling: `C/billingquantity.go:1057`; test `C/billingquantity_test.go:2310`. | **No.** The original operation must remain outstanding; a replayed server error must create its hold immediately. |

+| Astra F6 | **Closed for one sender releasing another’s lease.** | Conditional claim: `C/billingquantity.go:600`; token-qualified release: `:613`. Test: `C/billingquantity_test.go:2152`. | **No.** It checks both exclusion of a second live claimant and refusal of a nonowner release. |

+| Astra F7 | **Closed for recurrence of an earlier operation key.** | Persistent serial: `C/billingquantity.go:508`, `:549`. Test: `C/billingquantity_test.go:2235`. | **No.** Repeated former key components must still produce distinct keys. |

+| Astra F8 | **Partly closed.** The intermediate check now runs, but the oracle still hides some data and ownership violations. | `O/walk_sim_test.go:484`; oracle test `:200`. | **No** without the intermediate check. **Yes** with the remaining exclusions described below. |

+| Astra F9 | **Closed for the reported weak duplicate-execution assertion.** Provider behavior remains unverified. | Exactly one nonreplayed success: `B/sandbox_fence_test.go:299`; mixed-parameter races: `:308`. | **Not run.** The revised assertion rejects six honestly reported fresh executions. This direct-provider procedure does not test the console’s implementation. |

+| Astra R4, remaining portion | **Closed.** Historical ownership no longer counts as current holding. | Open-interval requirement: `O/walk.go:313`. Test: `O/ownership_test.go:1800`. | **No.** Both visible-bucket and record-only legacy-adopter cases require refusal. |

+| Opus N1 | **Partly closed.** Different-bucket replacement is refused; same-bucket replacement is still destroyed by the stale teardown. | `O/walk.go:756`, `:775`. Test: `O/ownership_test.go:1700`. | **No** without the bucket comparison. **Yes** while the same-bucket defect remains: the fixture changes A to B. |

+| Opus N2 | **Closed.** Grant recomputes its bucket from the fresh record and original arguments. | `O/walk.go:176`, `:215`. Test: `O/ownership_test.go:1847`. | **No.** Removing recomputation permits the stale adopted name. |

+| Opus N3 | **Partly closed.** Valid-name rotations now lease and fence; invalid-name records explicitly bypass both. | `O/walk.go:529`, `:608`, `:648`; bypass `:639`. Test: `O/ownership_test.go:1905`. | **No** without the valid-name protection. **Yes** with the invalid-name bypass; the fixture uses a valid name. |

+| Opus N4 | **Closed for the reported refused-fence review loop.** The new review authority has a separate identity defect. | `C/billingquantity.go:718`, `:1264`. Test: `C/billingquantity_test.go:2021`. | **No.** It checks suppression while open and progress after resolution. |

+| Opus N6 | **Partly closed.** The simulation now generates pauses, takeovers and additional faults, but its coverage and oracle remain incomplete. | `O/walk_sim_test.go:447`, `:484`; tests `:169`, `:200`. | **Potentially yes** if pause or fault-generation branches are removed: generating fewer adverse schedules need not fail any assertion. |

+| Opus N8 | **Closed for the reported deployment-store writes and unconditional walk deletes.** | `R/store.go:555`; `O/walk.go:1169`, `:1188`. Tests: `O/ownership_test.go:1666`, `:2067`; `S/sigfence_test.go:19`. | Conditional-delete tests **fail** without their protection. Signer tests **pass** if only production constructor wiring is removed. |

+| Opus N11 | **Closed for clock-skew safety under the stated backend window.** | `O/fence.go:49`, `O/walklock.go:54`. Test: `O/ownership_test.go:2029`. | **No** with the previous five-minute margin. The test does not independently model backend time or future-signature rejection. |

+| Opus N12 | **Partly closed.** Both policies, a test card, replay evidence and a delayed request are now covered. Declined-payment outcomes remain untested. | `B/sandbox_fence_test.go:126`, `:139`, `:206`, `:299`. | **Not run.** A successful immediate-payment run does not exercise a declined payment’s saved outcome. |

+| Opus N13 | **Partly closed.** Destruction is recorded earlier, but a crash between the two persistence writes still loses that history on take-back. Automatic retry remains explicitly outstanding. | Store note: `O/walk.go:921`; ledger write: `:928`; `O/ownership.go:519`. Test: `O/ownership_test.go:1967`. | **No** without the ledger write. **Yes** with the crash gap: the test fails at `DeleteUser`, after both writes succeeded. |

+Two remaining defects need concrete treatment:

+- **N1:** T1 reads store A and pauses before acquiring its lease. T2 destroys it; a grant recreates the same bucket name with a new key and new data. T1 resumes. At `O/walk.go:756` it replaces its original identity with the new record’s identity, and `:775` compares only the bucket name. It destroys the replacement. Preserve and compare the original estate identity before any destructive action, including the identity the console confirmed at `C/appsettings.go:990`.

+- **N13:** A has a retained teardown. B deletes their shared bucket and persists `BucketDeletedAt`, then crashes or fails before `markBucketDestroyed`. B’s subsequent grant clears the store note at `O/walk.go:238` and the ledger’s destroying marker at `O/ownership.go:345`. A can no longer discover that its objects were destroyed. Recover the ledger’s destruction record from the persisted note before either retry or take-back discards it.

+## 2. New findings

+### F1. P1 — Prefix purge bypasses the conditional protection for a declaration-only store

+**Where:** `R/trash.go:212`, `:229`, `:237`; `C/ownership.go:441`, `:502`.

+**Scenario:** A grant request is admitted while the app is live, then pauses before writing its grant block. Another process trashes and purges the app. `PurgeApp` reads the declaration-only `store.json`, passes its estate guard and pauses.

+The admitted grant resumes, takes the bucket lease, provisions the bucket and credentials, and records success. Purge then lists the prefix and unconditionally deletes its objects, including the new store record and delivered configuration. No bucket lease or conditional deletion protects this path. The estate remains without its cleanup record.

+The console’s process-local lock does not coordinate the two processes.

+**Smallest acceptable fix:** Coordinate purge with grant across processes and make the declaration removal conditional on the generation actually checked. A successful stale preflight must not authorize an unconditional prefix sweep over newly provisioned estate. Add a regression with an admitted grant completing between the purge’s store guard and deletion.

+### F2. P1 — Invalid bucket names select an unfenced authority path

+**Where:** `O/walk.go:639`, `:740`, `:845`, `:887`; record reader `O/objstore.go:179`.

+**Scenario:** A legacy or malformed record has a nonempty bucket name rejected by `ValidBucketName`, but names a real IAM user and keys. The reader accepts that record.

+`leaseForRotation` explicitly returns the original context and a no-op release function for this shape. Two rotations can therefore run concurrently: each reads K1, one delivers K2, and the other’s `ReapKeys(keep=K1)` deletes K2 before its eventual record conflict is detected. Teardown likewise skips the lease and ownership checks and proceeds with destructive provider calls.

+Rejecting such names in new grants does not protect records already present.

+**Smallest acceptable fix:** Refuse provider mutations for an unsupported bucket identity, or support legacy identities through a safe lease key and the same fenced context. Never interpret failed validation as permission to run without coordination. Cover both rotation verbs and teardown.

+### F3. P1 — A resolved review can authorize retirement of a different operation

+**Where:** `C/billingquantity.go:106`, `:725`, `:735`, `:1243`, `:1263`.

+**Scenario:** Operation K1 receives an unsettled review, which a person resolves. K1 is retired. A later K2 has the same creation second and subscription but a different serial and idempotency key—for example, after a clock returns to an earlier second, the case the serial change explicitly supports.

+K2 needs settlement. Its review lookup uses only account, subscription and `billing_quantity_operation_unsettled:<created_at>`. It finds K1’s resolution and marks K2 `reviewed`, without fencing K2 or obtaining a decision about it. A following pass can retire K2 and issue a replacement while K2’s delayed request remains executable.

+This does not require expiration of Stripe’s key retention. The worker’s disabled switches currently contain the path.

+**Smallest acceptable fix:** Bind the review to the persisted operation serial or full idempotency key, and verify that identity transactionally when consuming a resolution. Test K1’s resolution followed by K2 with the same creation second.

+The review test at `C/billingquantity_test.go:2021` and serial test at `:2235` test the two mechanisms separately and miss their interaction.

+### F4. P2 — A replayed metadata-fence server error has no review-based recovery

+**Where:** `C/billingquantity.go:1277`; contrast `:1057`.

+**Scenario:** Stripe saves a 500 response for the metadata fence. Subsequent fence attempts replay that result. `fenceQuantityOperation` leaves `FencedAt` unset and files reviews only for qualifying 4xx responses. It ignores the replay evidence on this 500.

+The already-documented stuck fence therefore does not receive the newly introduced operation review that would let a person resolve it. It remains outstanding and is retried for however long the provider retains the result. Errors reach the pass’s logs, but no operation-specific billing review exposes the recovery action.

+**Smallest acceptable fix:** File the unsettled review for a replayed server error from the fence itself, retain uncertainty, and suppress further attempts while that review is open. Test this separately from a replayed server error from the quantity update.

+### F5. P2 — The quantity simulation treats any open review as proof that outstanding work is safely held

+**Where:** `C/billingquantity_sim_test.go:309`, `:325`; skipped checks at `:341`, `:344`.

+**Scenario:** An earlier operation creates a dating review. A later operation becomes stuck without its own unsettled review. During settling, pending additions cause the simulation to inspect reviews. Any open review for the account immediately returns success.

+That bypasses the final provider/billing equality, member coverage and outstanding-operation checks. An unrelated historical review can therefore hide a new unrecoverable operation.

+**Smallest acceptable fix:** Require a review belonging to the blocked operation and verify that operation’s waiting state. Keep independent safety assertions active. Add an oracle regression containing an unrelated review and an unfenced outstanding operation.

+### Simulation coverage and remaining storage-oracle defects

+The **storage simulation** now generates nested grant/teardown calls, same-app nested calls after a simulated freeze, takeover beyond the lease, provider failures before and after effects, and store `Put` failures before and after effects.

+It still does not generate independent clocks, SDK retries, asynchronous request arrival, rotations, multiple bucket names, legacy records without intervals, or scanner faults.

+Its remaining F8/N6 weaknesses are concrete:

+- A successful grant rewrites its sentinel object at `O/walk_sim_test.go:269`, even when the store was already live. The intermediate check skips the outer app at `:336`. A faulty healthy re-grant can lose its own data and have the harness restore it before checking.

+- The oracle test at `:200` deliberately damages only another app’s object (`:476`), so it passes despite that hole.

+- Intermediate checks call `check()` but not `owner()`. Ownership snapshots and checks resume after the outer walk at `:516`.

+- User existence stands in for usable credentials and effective access. Neither valid keys nor delivered configuration are established by those checks.

+The **quantity simulation** now generates remembered server errors before and after an effect, alongside membership changes, webhooks, process interleavings and delayed requests. It still lacks independent clocks, review resolution, metadata-fence saved 500s, declined payments, key eviction and independently scheduled provider reconciliation of an indeterminate request. F5 further weakens its final liveness check.

+### Other requested boundaries

+**Signer coverage and requests without a fence.** The provisioner’s S3/IAM clients and `release.NewS3Store` now install the shared signer and watcher. The examined paginator paths retain their client and context. No multipart uploader or waiter was found in these walks. No additional concrete unsigned mutation or retry bypass was established for valid-name walks.

+Without a fence, `S/sigfence.go:79` preserves the supplied signing time. Without tracked doubt, the watcher has no walk state to modify. The ordinary retryer remains unchanged.

+Disabling second URI escaping is appropriate for the S3 operations used here: GetObject, PutObject, DeleteObject and ListObjectsV2. However, `S/sigfence_test.go:110` compares the replacement with the general signer, not the SDK’s default S3 signer across those operations. Full SDK-option parity cannot be established from the repository’s tests alone.

+**Thirty-minute lease.** Under the stated symmetric fifteen-minute backend window, the skew argument survives a behind taker, an ahead holder, and both clocks being wrong. With old stamp \(S\), new stamp \(N\), and window \(W\), expiry takeover requires \(N \ge S+2W\). Acceptance of the new request at backend time \(T\) requires \(T \ge N-W\), hence \(T \ge S+W\). This is an argument about backend acceptance, conditional on that window—not something the single-clock simulation proves.

+**Sandbox procedure.** The revised replay assertion rejects the former six-executions counterexample, and mixed-parameter races add useful coverage. A pass remains finite evidence; it may exercise only one winning ordering, which `B/sandbox_fence_test.go:342` merely logs. Immediate mode exercises a successful card, not declined-payment outcomes.

+The canceled-subscription check accepts any 4xx at `:354`, including authentication or rate-limit failures, without rereading quantity. Such a response does not establish that cancellation itself prevents the update.

+Cleanup can leave sandbox objects behind: products and prices are archived, and cleanup HTTP statuses are ignored at `:114`, `:118`, `:137`, `:149`. No concrete live-mode path was found: the procedure requires `rk_test_`, uses the fixed API host and refuses redirects.

+**Disabled paths.** No bypass of the two quantity switches was found: `C/billingquantity.go:128` and `:813` gate processing before discovery or fencing. No production attestation-enabling call was found. The two defects explicitly excluded by the brief are not repeated as findings.

+## 3. Claims the code contradicts or does not establish

+| Location | Claim | What the code supports |

+|---|---|---|

+| `implementation/carloku-paid-launch/AGENT_HANDOFF.md:35`; `OUTSTANDING.md:65`; `O/walklock.go:29` | Walks hold a bucket lease and every request carries its stamp. | Invalid-name rotations and teardowns deliberately bypass both. Prefix purge also remains outside the walk protection. |

+| `AGENT_HANDOFF.md:35` | A retained teardown finishes once its bucket has been destroyed. | It requires another invocation, and the N13 persistence gap can discard the evidence needed by that invocation. |

+| `C/billingquantity.go:715` | A resolved review belongs to “that very operation.” | The lookup identifies a creation second, not the serial or key. F3. |

+| `AGENT_HANDOFF.md:38`; `C/billingquantity.go:649`; evidence document’s opening fence description | Every unconfirmed acknowledgment first consumes the key at the provider. | The human-reviewed path can acknowledge without consuming a provider key. That exception needs to be stated explicitly. |

+| `C/billingquantity.go:658` | Retirement always follows a provider read taken after the fence. | Ended-subscription processing and changed-subscription abandonment do not necessarily obtain that read. |

+| `OUTSTANDING.md:66` | Every operation that cannot be settled has a hold and a way out through a resolved review. | Replayed metadata-fence 500s create no such review. F4. |

+| `O/fence_aws_test.go:29`, `:122`; `OUTSTANDING.md:71` | The live probe establishes retry behavior. | Its retry portion logs `doubt.Raised()` but never asserts it at `:129`. A refusal without a retry passes. The separate local SDK test does assert two attempts. |

+| `O/fence_aws_test.go:111` | The escaped-key probe requires the operation’s own refusal. | At `:118`, any modeled API error except `SignatureDoesNotMatch` passes, including errors other than the stated `NoSuchKey` or `AccessDenied`. |

+| `B/sandbox_fence_test.go:33` | Created objects are canceled and deleted when the test ends. | Product and price are archived; cleanup failures are not checked. |

+| Evidence document `:112` | Canceled-subscription behavior is “Logged, not asserted.” | `B/sandbox_fence_test.go:354` requires a 4xx, and `:368` also asserts fence consistency. |

+| `AGENT_HANDOFF.md:64` | Customer payment of the wrong finalized invoice is undetermined. | `OUTSTANDING.md:39` and the evidence document now explicitly classify it as customer-payable exposure. The handoff retains the superseded uncertainty. |

+The evidence document correctly labels Stripe behavior unverified. `OUTSTANDING.md:71` also correctly distinguishes observed read-only AWS calls from the inference that the same window protects writes and deletes. Neither establishes the complete walk invariant.

+**Verdict: reject.**

\ No newline at end of file

diff --git a/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round8.md b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round8.md

new file mode 100644

index 000000000..edd9419b7

--- /dev/null

+++ b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round8.md

@@ -0,0 +1,249 @@

+# Opus production and authority review, round 8: Carloku paid launch, `aed42815..c3e87a94`

+I read `43383b93..c3e87a94` as whole files at `c3e87a94`, using `git show` only. This was a read-only pass: no tests, no mutations, no network calls. Every answer to "would the test fail without the fix?" comes from reading the code. To check the S3 signer question I read the pinned SDK sources in the local module cache (`aws-sdk-go-v2 v1.44.0`, `service/s3 v1.105.0`).

+**Production authority**

+- **Nothing attests.** Nothing under `internal/host`, `internal/usage`, `internal/ops` or `cmd` changed since round 7. `SetAttestation` and `SetAllClientsInstrumented` are only defined (`usage/durabilityledger.go:1455`, `:1475`); nothing outside tests calls them.

+- **The quantity worker cannot reach Stripe with either switch unset.**

+ - Only two non-test calls go to the provider: `UpdateQuantityWith` (`billingquantity.go:1049`) and `FenceQuantityKey` (`:1277`). Both are reached only through `billingQuantityPass`.

+ - `billingQuantityPass` returns before any read unless `quantityWorker()` sees both exact values (`:813-816`, `:128-134`). Malformed values are refused at startup (`cmd/console/billing.go:37-42`).

+ - `UpdateQuantity` has no non-test caller.

+- **Billing a customer wrongly.** I found no reachable path:

+ - Every acknowledgement carries `quantityOperationSettled` or `quantityOperationRetirable` (`:247`, `:285`, `:630`, `:775`).

+ - The invoice engine still cannot freeze a month while replication completeness is unprovable.

+- **Leaks and data.** Neither new code path logs a key, a response body or a Stripe identifier. One narrow race can still destroy a customer's store: the remaining half of round 7's N1 (N1 below).

+## 1. Disposition table (round 7)

+Abbreviations: **O** = `internal/objstore`, **C** = `internal/console`, **B** = `internal/billingstripe`, **S** = `internal/sigfence`.

+|---|---|---|---|

+| Astra F1: declaration-only teardown erases a store granted meanwhile | **Closed in code.** The S3 backend's refusal is unverified (§3). | `O/walk.go:1169` (`DeleteIfMatch`), `:716`, `:779` | `ownership_test.go:1666`: **fails** with an unconditional delete. It runs on DirStore only. |

+| Astra F2: teardown holds A's lease and acts on a record naming B | **Closed when the name differs.** Not closed when the same name was re-minted (N1). | `O/walk.go:768-777` | `:1700`: **fails** without the comparison. |

+| Astra F3: deployment-bucket writes are not fenced | **Closed in code. Nothing tests that the production clients carry the fence (N5).** | `release/store.go:555-556`; sidecar delete `O/walk.go:1045`, `:1180-1189` | `:2067` **fails** with an unconditional sidecar delete. No test fails if the signer is removed from `NewS3Store` or `NewAWSProvisioner`. |

+| Astra F4: a successful retry frees the name | **Closed for the walk's own release.** A same-app, same-kind takeover can still overlap an ended walk's late requests (N2). | `S/sigfence.go:401-433`; `O/walklock.go:200` | `S/sigfence_test.go:53` uses a real SDK client. `ownership_test.go:1761` **fails** without the doubt check. |

+| Astra F5: an indeterminate 500 is retired as settled | **Closed for automatic retirement.** A person's resolution still retires it, by design. | `C/billingquantity.go:704-707`, `:1221-1250`; `B/client.go:675-736` | `billingquantity_test.go:2089` **fails** if `request` is made retirable. |

+| Astra F8 / Opus N6: the simulations | **Partly closed.** Storage: mid-walk checks, freezes past the lease, failures after the effect, store faults. Still missing: a pause between the first read and the lease, a second name, a second clock. Quantity: the Held and Settles bookkeeping is unchanged (N7). | `O/walk_sim_test.go:121-171`, `:445-489` | `walk_sim_test.go:200` shows the oracle sees a transient violation. |

+| Astra F9 / Opus N12: the sandbox procedure | **Partly closed. Not run.** Added: both policies, exactly one non-replayed run, a six-round fence/update race, a 90-second late update, raw statuses in check 6. Missing: a declined card, and `Idempotent-Replayed` on an error answer (N8). | `B/sandbox_fence_test.go:126-371` | n/a |

+| Opus N1 | **Partly closed** (see F2) | — | — |

+| Opus N7 | **Closed** (same fix as F4) | — | — |

+| Opus N11: takeover trusts the taker's clock | **Closed by the doubled margin.** A far-ahead clock can still wedge a name (N3). | `O/fence.go:259-269`; `walklock.go:54` | `ownership_test.go:2029` **fails** with a 15-minute lease. |

+| Opus N13: a take-back does not record the destruction | **Closed.** A retained teardown still finishes only when someone re-runs it, as `OUTSTANDING.md` 19a says. | `O/walk.go:927-931` | `:1967` **fails** without `markBucketDestroyed`. |

+## 2. New findings, most severe first

+### N1. P2: a stale teardown destroys a store re-created on the same minted name

+**Where:** `O/walk.go:700-777`. The values from the first read (`startKeyID`, `startBucket`, the grant instant) are overwritten from the re-read at `:756-762`. The only comparison is the bucket name (`:775`).

+**Scenario:**

+1. A member's destroy request runs Teardown T1 in process P1. T1 reads `bab/x`: Grant `{A, minted}`, KeyID K1. P1 then stalls before `acquireBucketWalk`. A VM suspend works: Go's timers do not advance across it.

+2. The member retries and lands on P2. T2 finishes the teardown, and A's lease is freed cleanly.

+3. The member creates the store again. The grant mints the same deterministic name A, delivers K2, and the app writes data.

+4. T1 wakes, takes lease A (it is free) and re-reads. The record still names A, so the check passes.

+5. T1 writes its teardown block, wins `beginBucketTeardown` (the app holds A alone), empties A, deletes A and deletes the user. The new store and its data are destroyed by a request made against the store before it.

+The lease and the fence are both satisfied. Only the request is stale. Neither test nor simulation generates this: the simulation never pauses between the first read and the lease.

+**Smallest fix:**

+- Keep the estate from the first read: `Provisioned.KeyID`, `Grant.At`, and whether a teardown block existed.

+- After the re-read under the lease, refuse with `ErrCASConflict` if any of them differs. Only then write anything.

+- Test: tear down, re-mint the same name, then resume the stale teardown.

+### N2. P3: the lease is on the bucket name, but the IAM user belongs to the app, and a same-kind takeover overlaps the earlier walk's late requests

+**Where:**

+- `O/walklock.go:129-137`: an ended lease is taken at once by the same app re-running the same kind of walk.

+- `:203-209`: `Carry` keeps only that *name* reserved.

+- `UserName(acct, app)` (`O/walk.go:72`) is the same for every store the app ever has.

+**Scenario:**

+1. App X has an *adopted* bucket A. Teardown T1 ends with an error while its `DeleteUserPolicy(U)` is still in transit, stamped at T1's lease instant.

+2. T2, the same app's teardown re-run, takes lease A immediately and finishes. The name stays reserved until T1's end.

+3. The member re-declares, and the grant mints M ≠ A. Lease M is free. The grant creates user U, writes the policy for M, mints a key and records a healthy store.

+4. T1's delayed `DeleteUserPolicy(U)` arrives within its 15 minutes and is accepted. U loses its only policy.

+The store is dark, and its record reads healthy. Nothing runs Grant again to repair it. No data is lost and nothing leaks.

+**Smallest fix:** each walk also takes a lease keyed on the app's user name, for example `objstore/walks/user-<acct>-<app>.json`, with the same `Carry`.

+### N3. P3: a far-ahead clock wedges a bucket name, and `Carry` passes the wedge on

+**Where:** `O/walklock.go:129-140` and `:203-209`.

+**Scenario:**

+1. One console's clock is a day ahead.

+2. Its walk writes `Until` a day ahead. AWS refuses every fenced request, so the walk fails and the lease stands.

+3. Every other taker is busy until that instant.

+4. The same app's retry may take it, but inherits `Carry` = that `Until`. Even a clean finish sets `Until = Carry`.

+The name stays reserved for a day, and there is no operator door to clear it.

+This attack on the thirty-minute argument ends in a hold, not a safety failure. I checked the other cases with holder offset h and taker offset k against AWS's ±15-minute window: the taker takes over at T0+h−k+30, and the holder's requests stop being accepted at T0+h+15. That is safe whenever k ≤ 15. When k > 15 the taker's own fenced stamps are refused. Both clocks wrong at once does not change this.

+**Smallest fix:** treat a lease whose `Until` or `Carry` is more than `walkLease` past the taker's now as corrupt. Refuse with a distinct error that names it, and give an operator verb to clear it.

+### N4. P3: a resolved review of one operation can retire a later operation without any fence

+**Where:**

+- The review reason is `billing_quantity_operation_unsettled:<CreatedAt>` (`C/billingquantity.go:106-108`), keyed by account, subscription and reason.

+- `markQuantityReviewed` (`:725-736`) and `fenceQuantityOperation`'s resolved branch (`:1270-1272`) accept *any* resolved row with that reason.

+**Scenario:**

+1. K1's review is resolved, and K1 is retired.

+2. K2, on the same subscription, gets the same `CreatedAt`. This happens when `created = max(clock, PeriodStart, NewestAt)` is pinned by a clock behind `NewestAt` or behind the period start. The serial comment at `:502-507` concedes this recurrence.

+3. On K2's first fence attempt the resolved branch marks it `reviewed`. No fence request is ever sent.

+4. K2 is retired and replaced while its own request may still be in flight under a key Stripe never saw consumed.

+**Smallest fix:** put the operation's serial or key in the reason, and match on it in both places.

+### N5. P3: nothing tests that the production clients carry the fence or the attempt watcher

+**Where:** `release/store.go:555-556` and `O/aws.go:83-84`, `:99-100`.

+- `fence_aws_test.go:49`, `:61` build their own clients with the options inline.

+- `S/sigfence_test.go` builds its own client too.

+Removing either pair of lines from the production constructors fails no test. This is the mechanism ruling 12 rests on, and the round 7 dispositions' mutation list does not cover it.

+**Smallest fix:** a test that builds `release.NewS3Store` and `NewAWSProvisioner` against an `httptest` endpoint with static credentials, then asserts:

+- under `withFence`, `X-Amz-Date` is the fence instant;

+- under a plain context, it is now;

+- a 500-then-200 sequence raises doubt.

+### N6. P3: a record with an invalid bucket name gets teardown and rotation with no lease and no fence

+**Where:**

+- Teardown: `O/walk.go:740` skips the lease, and `:845-849` sets `destroy = true` with no holder check.

+- Rotations: `:639-647` runs unleased.

+Both reach `DeleteUser`, `ReapKeys` or `MintKey` on the per-app user, and teardown also reaches `EmptyBucket` and `DeleteBucket`.

+Grant refuses such names, so only legacy or hand-edited records qualify. The repository does not say whether any exist. A stale unleased rotate on such a record reaps a later valid store's key: round 7's N3 again, for these records.

+**Smallest fix:** refuse to walk an invalid name and send it to an operator path. At least take the app-user lease from N2.

+### N7. P3: simulation bookkeeping that can hide a real violation

+- **Quantity, Held** (`billingquantity_sim_test.go:223`): accepts *any* open `billing_quantity_date_unproven:%` review on the account, not the one for the operation behind each `landed_unverified` addition. Astra raised this in round 7, and it is unchanged.

+- **Quantity, Settles** (`:327-330`): passes pending additions if *any* review is open, including an unrelated mismatch or uncharged-period review. A wedge that files nothing is therefore invisible once anything else is open.

+- **Quantity, reviews:** the simulation never resolves a review, so the new way out (resolve, mark reviewed, retire, replace) never runs under random schedules. The indeterminate fault is never reconciled later.

+- **Storage, after a fenced grant** (`walk_sim_test.go:249`): a frozen outer *grant* that wakes and is refused by the fence relabels its app `broken`. That switches off the Gone checks for the app the frozen schedule exists to test. Any leftover is visible only if the settle re-run fails to clean it.

+- **Storage, model limits:** it freezes only at provider calls inside the lease, never between the first read and the lease (N1). It runs on one clock (`walkClock = at`) and one name.

+**Fix:**

+- Match reviews by operation.

+- Excuse a pending addition only with that operation's unsettled review.

+- Add a "person resolves" step.

+- Keep the outer app's state across a fenced return.

+- Add a pause before `acquireBucketWalk`.

+### N8. P3: `TestSandboxQuantityFence`, read as the procedure

+**Could it pass against a provider that does not fence?** No.

+- Check 1 needs the late update refused with 400 `idempotency_error` and the quantity unchanged.

+- Check 3 needs a fresh fence after a validation refusal, then the update refused.

+- Check 5 fails whenever both or neither take the key.

+- Check 4's `ran == 1` closes F9's "six absolute updates" gap.

+**Would a pass under `immediate` establish what the gate needs?** Only for a card that succeeds (`pm_card_visa`).

+- A declined card, or one needing 3DS, under `always_invoice` is never exercised. What Stripe saves under the key, and whether the quantity moves, are unknown. The worker records `charged` on any 200.

+- The worker's replayed-500 branch (`C/billingquantity.go:1057`) relies on `Idempotent-Replayed` on an *error* answer. Check 2 only observes it on a 200. If Stripe omits it on errors, the branch never fires, and the operation falls back to the 22-hour path. That fails safe.

+- In check 5, when the update wins against a 409 fence, the fence is never re-sent to confirm it then reads `FenceHeldByRequest`.

+**Live mode and secrets:** it cannot touch live mode. It requires `rk_test_`, uses the fixed host, refuses redirects, and never logs the key or a body.

+**Objects left behind:** products and prices are archived, not deleted. Invoices and charges from the `immediate` run remain in the sandbox. The comment at `:33-34` says "cancels and deletes them".

+**Hygiene:** `raw` closes over the parent `t` and calls `Fatalf` from goroutines (checks 4 and 5). That produces a failure, never a false pass.

+**Fix:**

+- Add a declined-card (`pm_card_chargeDeclined`) round under `immediate`.

+- Add a replayed-error probe, for example a saved 402.

+- Correct the comment.

+- Use a goroutine-safe error channel.

+### Asked and not found

+- **Doubt.** I found no retried attempt that escapes it. Paginators and `ReapKeys` loops are separate operations, each watched. No waiter or multipart call is used. Go's transport replays only GET/HEAD here: AWS requests carry no `Idempotency-Key` header. I found no path that frees the name while doubt stands: `release` checks it on every path, including `leaseForRotation`'s early `release(true)`.

+- **Fence reach.** Every client a walk reaches carries the signer. Installing it on every `NewS3Store` user is harmless:

+ - without a fence the signer passes through the SDK's skew-corrected time;

+ - the watcher is a no-op when the context carries no doubt;

+ - a stack with no `Retry` step is skipped.

+- **The S3 signer's escaping matches the SDK's.** `newDefaultV4Signer` sets `DisableURIPathEscaping = true` (`s3@v1.105.0/api_client.go:716`). The auth adapter also sets it per request from the endpoint's `DisableDoubleEncoding` property (`aws-sdk-go-v2@v1.44.0/internal/auth/smithy/v4signer_adapter.go:47`), which overrides the constructor for every operation either way. The constructor option is redundant and harmless.

+- **Send lease.** Two senders can both be live only if their clocks differ by more than `billingClockTolerance`. Both then send the same key, which is harmless.

+## 3. Claims the code contradicts

+**`walklock.go:42-44` and `fence.go:252-257`**

+- The claims: "a lease cannot be taken over while any request of the walk that held it could still be accepted", and "by the time another walk may take the name, every request the earlier walk could ever send is one the backend refuses".

+- `acquireBucketWalk:129-137` lets the same app re-running the same kind take an ended lease at once. `Carry` protects only the name (N2).

+**`walklock.go:29-30`, and `AGENT_HANDOFF.md` item 5 ("Grant, teardown and both rotation verbs are walks … every request of a walk … is signed with the instant its lease was taken")**

+- Records with invalid bucket names run with no lease and no fence (N6).

+- "AWS refuses …" holds for AWS endpoints only. An S3-compatible deployment bucket or store endpoint (`CARLOS_S3_ENDPOINT`, `Deployment.Endpoint`) was never probed for its signing-time window.

+**Round 7 dispositions (F1 row: "the backend refuses the delete"), and `OUTSTANDING.md` "Tested" ("deletes conditional on the generation read")**

+- S3 `DeleteObject` with `If-Match` has been exercised only against DirStore and an HTTP stub (`release/conditional_delete_test.go`). Nothing has sent it to AWS or to Hetzner RGW, whose `If-Match` quirk is documented for PUT only (`s3compat.go:122-150`).

+- A backend that ignores the header deletes unconditionally.

+**`AGENT_HANDOFF.md` item 5, "finishes once that bucket has been destroyed"**

+- Only when someone re-runs it. `OUTSTANDING.md` 19a says so; the handoff does not.

+**`billingquantity.go:1057-1064` and the dispositions ("Stripe's `Idempotent-Replayed` header") for a replayed server error**

+- Not documented for error answers in the evidence document, and not observed by the sandbox procedure (N8).

+**`OUTSTANDING.md` "Tested": "every request to S3, IAM and the deployment bucket stamped with the lease instant"**

+- Tested on hand-built clients, not on the production constructors (N5).

+**Dispositions mutation list: "teardown not comparing its record with its lease" is caught**

+- Caught only when the name differs. Dropping the comparison of the estate itself is untested, and the code does not make that comparison at all (N1).

+**`sandbox_fence_test.go:33-34`, "cancels and deletes them"**

+- Products and prices are archived. Invoices and charges remain.

+**The evidence document and `OUTSTANDING.md` items 7 and 9**

+- I found nothing that makes either defect reachable with both switches unset and billing off, and nothing in either document that the code contradicts.

+- The late-request design's statement that Go's transport re-sends POSTs carrying `Idempotency-Key` is accurate for this snapshot's client, which uses the default transport. That re-send is part of what the design removes, and is not reachable today.

+## 4. Verdict

+**Accept with fixes.**

+- Nothing attests. The quantity worker cannot reach Stripe with either switch unset. Billing stays off.

+- **Before the storage changes land:** N1 (P2), with a same-name re-mint reproduction.

+- **Before the claims in §3 are relied on:** N2 to N6.

+- **Before `CONSOLE_BILLING_QUANTITY_FENCE` is set:** N4, and N8's declined-card round.

diff --git a/implementation/carloku-paid-launch/AGENT_HANDOFF.md b/implementation/carloku-paid-launch/AGENT_HANDOFF.md

index 84e7916b7..f9be6a6f6 100644

--- a/implementation/carloku-paid-launch/AGENT_HANDOFF.md

+++ b/implementation/carloku-paid-launch/AGENT_HANDOFF.md

@@ -32,7 +32,7 @@ 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 takes a lease on the bucket name (`walklock.go`): 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 lead to the leased name. 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: 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), and finishes once that bucket has been destroyed. `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 (same grant, same key), not only the same name. 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: 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. `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.

@@ -61,7 +61,7 @@ The monthly freeze requires all five billed meters complete. Two cannot be, in p

- A store's partial first hour holds the month if it has a charged line, and nothing releases the hold. Needs a decision before the AWS principal inventory lands (collector plan).

- A box whose ledger opened after the box began serving holds that day for everyone. Whether a box's first minutes may be waved through is undecided, so they hold.

- The quantity fence is unverified against Stripe and is not a permanent token (item 8). A sender frozen for more than a day can still land a request; that is detected and held, not prevented.

-- The invoice engine's lease has no provider-side fence beyond identical replays under one key. Past Stripe's key memory a late request runs as new and is detected and held. One schedule leaves an open invoice with a wrong total that the engine refuses to charge; whether the customer could pay it from Stripe's hosted page is not determined.

+- The invoice engine's lease has no provider-side fence beyond identical replays under one key. Past Stripe's key memory a late request runs as new and is detected and held. One schedule leaves an open invoice with a wrong total that the engine refuses to charge. An open invoice can be paid by the customer from Stripe's hosted page, so this is customer-payable exposure and an open defect (`OUTSTANDING.md` item 9). Billing is off.

- Rolling back to a build from before `5f2b4faf` on a database that has recorded a `landed_unverified` outcome marks those accounts `history_unproven`, a durable hold: the older build's audit sweep does not know the outcome. Nothing has recorded one (the proration switch is unset).

## Verification in this run

@@ -77,7 +77,7 @@ Merchant `acct_1UD7LXI4Nrr6AEsO`, Team Tito Limited, Ireland, EUR; Irish standar

## Next actions, in order

1. Apply approved copy from both reviews when `result.json` arrives.

-2. Independent review: Astra rejected rounds 1 to 7 (ten, eight, five, five, eleven, fourteen and nine findings); Opus accepted with fixes six times (eight, eight, eight, nine, nine, thirteen). Round seven attacked the lease and the fences at their edges and every finding was accepted (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, "Round seven"). Round eight of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted".

+2. Independent review: Astra rejected rounds 1 to 8 (ten, eight, five, five, eleven, fourteen, nine and five findings); Opus accepted with fixes seven times (eight, eight, eight, nine, nine, thirteen, eight). Rounds seven and eight attacked the lease and the fences at their edges; every finding was accepted, and two of round eight's are recorded as not fixed (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, "Round eight"). A further round of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted".

3. Scoped approvals: Cost and Usage Report export; Stripe catalog, portal, webhook and the `rk_live` key; then a test-mode rehearsal and a real purchase.

4. Build the principal inventory and the bucket ownership attestation (designed in `docs/superpowers/plans/`), quantity reduction at renewal, a `carlos` verb and documentation for the operator doors, then Free-tier enforcement.

5. Build path B of the attestation plan (the platform's own replicator as sole writer). Until a box can prove its replication writes, that meter holds every month.

diff --git a/implementation/carloku-paid-launch/OUTSTANDING.md b/implementation/carloku-paid-launch/OUTSTANDING.md

index 7e3dab127..af2092257 100644

--- a/implementation/carloku-paid-launch/OUTSTANDING.md

+++ b/implementation/carloku-paid-launch/OUTSTANDING.md

@@ -36,7 +36,7 @@ Each is concrete. None is waived.

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.

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.** 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.** (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.

### Decisions that are Paul's

@@ -55,14 +55,17 @@ Each is concrete. None is waived.

### Process

-19. **Round eight review has not passed.** Astra rejected rounds one to seven; Opus accepted each with fixes. The fixes to round seven's findings are unreviewed.

+19. **No review round has passed.** Astra rejected rounds one to eight; Opus accepted each with fixes. The fixes to round eight's findings are unreviewed.

+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.

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.

## Tested on this branch, not proven

-- Bucket-name walks (grant, teardown, rotation): a lease per bucket name across processes, the record re-read under it, every request to S3, IAM and the deployment bucket stamped with the lease instant, deletes conditional on the generation read, the name held if any request needed a second attempt. All of it has run only against the in-memory provisioner and a directory store. The simulation covers sequential walks, walks frozen past their lease while another takes the name, and failures before and after the effect. It does not cover a second bucket name, rotations, stores with no ledger interval, or clocks that disagree; those have one direct test each.

+- Store walks (grant, teardown, rotation): a lease per bucket name across processes (per app when there is no usable name), the record re-read under it and required to be the same estate, every request to S3, IAM and the deployment bucket stamped with the lease instant (checked on the production constructors against a local server), deletes conditional on the generation read, the name held if any request needed a second attempt. The app purge never sweeps a store record it did not read as a bare declaration. The walks themselves have run only against the in-memory provisioner and a directory store. The simulation covers sequential walks, walks frozen past their lease while another takes the name, walks held before their lease while the store is replaced, and failures before and after the effect. It does not cover a second bucket name, rotations, stores with no ledger interval, or clocks that disagree; those have one direct test each.

- Quantity worker: an owned send lease, the key fence and its two kinds, keys that never recur, the hold on every unverified addition and on every operation that cannot be settled, the way out through a resolved review. All against a fake provider written from Stripe's documentation.

- Partial-month billing, statement folds, the transfer classifier (pinned against the flagship's real usage types), the invoice engine's recovery paths.

diff --git a/internal/billingstripe/sandbox_fence_test.go b/internal/billingstripe/sandbox_fence_test.go

index 4858432a8..6ec9e3c19 100644

--- a/internal/billingstripe/sandbox_fence_test.go

+++ b/internal/billingstripe/sandbox_fence_test.go

@@ -29,9 +29,13 @@ import (

// It runs only when CARLOS_STRIPE_SANDBOX_KEY holds a TEST-MODE restricted

// key (rk_test_...) with write access to customers, products, prices and

-// subscriptions and read access to invoices. It refuses any other key. It

-// creates one customer, product, price and subscription in the sandbox,

-// and cancels and deletes them when it ends. It never prints the key.

+// payment methods, subscriptions and read access to invoices. It refuses any

+// other key. It creates a product, a price, and a customer and subscription

+// per policy in the sandbox. When it ends it cancels the subscriptions,

+// deletes the customers and archives the product and the price (Stripe does

+// not delete a price that was used). The invoices and test charges the

+// immediate policy produced stay in the sandbox. Cleanup is best effort and

+// its failures are not checked. It never prints the key.

// What it cannot check: what happens after the provider has forgotten a

// key. Stripe documents that keys may be removed once they are at least 24

@@ -60,8 +64,10 @@ func TestSandboxQuantityFence(t *testing.T) {

// raw sends one request and returns what Stripe answered. Only the

// status, the error's type, code and param, and named fields are ever

// logged: never a body, never the key.

- raw := func(method, path string, form url.Values, idem string) answer {

- t.Helper()

+ // send is raw without the test's Fatal, for use from goroutines: a

+ // transport failure comes back as status 0.

+ var raw func(method, path string, form url.Values, idem string) answer

+ send := func(method, path string, form url.Values, idem string) answer {

var body io.Reader

if method == http.MethodPost {

body = strings.NewReader(form.Encode())

@@ -70,7 +76,7 @@ func TestSandboxQuantityFence(t *testing.T) {

}

req, err := http.NewRequestWithContext(ctx, method, defaultBaseURL+path, body)

if err != nil {

- t.Fatal(err)

+ return answer{}

}

req.Header.Set("Authorization", "Bearer "+key)

req.Header.Set("Stripe-Version", apiVersion)

@@ -82,7 +88,7 @@ func TestSandboxQuantityFence(t *testing.T) {

}

resp, err := httpc.Do(req)

if err != nil {

- t.Fatalf("%s %s: request failed (no body shown)", method, strings.SplitN(path, "?", 2)[0])

+ return answer{}

}

defer resp.Body.Close()

out := answer{status: resp.StatusCode, replayed: resp.Header.Get("Idempotent-Replayed") == "true"}

@@ -90,6 +96,14 @@ func TestSandboxQuantityFence(t *testing.T) {

_ = json.Unmarshal(rawBody, &out.body)

return out

}

+ raw = func(method, path string, form url.Values, idem string) answer {

+ t.Helper()

+ a := send(method, path, form, idem)

+ if a.status == 0 {

+ t.Fatalf("%s %s: request failed (no body shown)", method, strings.SplitN(path, "?", 2)[0])

+ }

+ return a

+ }

errOf := func(a answer) string {

e, _ := a.body["error"].(map[string]any)

return fmt.Sprintf("status=%d type=%v code=%v param=%v", a.status, e["type"], e["code"], e["param"])

@@ -275,7 +289,7 @@ func TestSandboxQuantityFence(t *testing.T) {

wg.Add(1)

go func() {

defer wg.Done()

- answers[i] = raw("POST", itemPath, updateForm(12, at), k("four"))

+ answers[i] = send("POST", itemPath, updateForm(12, at), k("four"))

}()

}

wg.Wait()

@@ -314,8 +328,8 @@ func TestSandboxQuantityFence(t *testing.T) {

var f, u answer

var wg sync.WaitGroup

wg.Add(2)

- go func() { defer wg.Done(); f = raw("POST", itemPath, fenceForm, key) }()

- go func() { defer wg.Done(); u = raw("POST", itemPath, updateForm(target, at), key) }()

+ go func() { defer wg.Done(); f = send("POST", itemPath, fenceForm, key) }()

+ go func() { defer wg.Done(); u = send("POST", itemPath, updateForm(target, at), key) }()

wg.Wait()

refusedOrBusy := func(a answer) bool { return mismatch(a) || a.status == 409 }

endQ := quantity()

@@ -329,6 +343,11 @@ func TestSandboxQuantityFence(t *testing.T) {

}

// Whatever a 409 refused must still be refused or

// replayed afterwards, never run.

+ if f.status == 409 {

+ if a := raw("POST", itemPath, fenceForm, key); !mismatch(a) {

+ t.Fatalf("round %d: the fence, retried after its 409 against an update that won: %s, want 400 idempotency_error", round, errOf(a))

+ }

if u.status == 409 {

if a := raw("POST", itemPath, updateForm(target, at), key); !mismatch(a) {

t.Fatalf("round %d: the update, retried after its 409 against a fence that won: %s, want 400 idempotency_error", round, errOf(a))

@@ -344,6 +363,48 @@ func TestSandboxQuantityFence(t *testing.T) {

}

})

+ t.Run("5b a card that declines, and whether a remembered error says it is remembered", func(t *testing.T) {

+ if mode.behaviour != ProrationAlwaysInvoice {

+ t.Skip("only the immediate policy charges a card")

+ }

+ // The subscription's card is replaced by one that attaches

+ // and then declines every charge.

+ pm := id(raw("POST", "/v1/payment_methods/pm_card_chargeCustomerFail/attach", url.Values{"customer": {customerID}}, k("card-fail")), "declining payment method")

+ if a := raw("POST", "/v1/subscriptions/"+subID, url.Values{"default_payment_method": {pm}}, k("card-fail-default")); a.status != 200 {

+ t.Fatalf("set the declining card: %s", errOf(a))

+ }

+ startQ := quantity()

+ // With error_if_incomplete the provider answers 402 and does

+ // not apply the update. The request began executing, so the

+ // answer is saved under the key.

+ form := updateForm(startQ+1, at)

+ form.Set("payment_behavior", "error_if_incomplete")

+ first := raw("POST", itemPath, form, k("declined"))

+ second := raw("POST", itemPath, form, k("declined"))

+ t.Logf("OBSERVED: a declined immediate update is answered %s, and again %s (replayed=%v)", errOf(first), errOf(second), second.replayed)

+ if first.status < 400 || first.status >= 500 {

+ t.Fatalf("a declined update with error_if_incomplete: %s, want a 4xx", errOf(first))

+ }

+ if q := quantity(); q != startQ {

+ t.Fatalf("a declined update with error_if_incomplete moved the quantity from %d to %d", startQ, q)

+ }

+ // The worker stops replaying a remembered server error by

+ // reading the Idempotent-Replayed header on an ERROR answer.

+ // A remembered 500 cannot be produced on demand; a remembered

+ // 402 is the nearest thing that can.

+ if second.status != first.status || !second.replayed {

+ t.Fatalf("the second copy of a failed request: %s replayed=%v, want the same answer marked as replayed. Without the mark on error answers the worker's hold on a replayed 500 never fires, and such an operation waits out the 22-hour limit instead", errOf(second), second.replayed)

+ }

+ if kind, err := c.FenceQuantityKey(ctx, item, 55, k("declined")); err != nil || kind != FenceHeldByRequest {

+ t.Fatalf("the fence under a key whose request failed: kind=%q err=%v, want %q", kind, err, FenceHeldByRequest)

+ }

+ // And what the worker's own request does with a declining

+ // card (no payment_behavior: the provider's default).

+ werr := update(startQ+1, at, k("declined-default"))

+ t.Logf("OBSERVED: the worker's own update with a declining card returns err=%v; quantity %d -> %d. The worker records an addition as charged on any success, so a success here with an unpaid invoice is a finding for the worker, to be read from this log", werr, startQ, quantity())

+ t.Logf("VERIFIED: a failed request's answer is saved and replayed with Idempotent-Replayed, and its key reads as held by the request")

+ })

t.Run("6 a canceled subscription", func(t *testing.T) {

before := quantity()

if a := raw("DELETE", "/v1/subscriptions/"+subID, nil, ""); a.status != 200 {

@@ -351,8 +412,14 @@ func TestSandboxQuantityFence(t *testing.T) {

}

canceled = true

u := raw("POST", itemPath, updateForm(before+1, at), k("six-update"))

- if u.status < 400 || u.status >= 500 {

- t.Fatalf("a quantity update on a canceled subscription's item: %s, want a definite refusal", errOf(u))

+ // A refusal about the request itself. An authentication,

+ // permission or rate-limit answer says nothing about what a

+ // canceled subscription does.

+ if u.status != 400 && u.status != 404 {

+ t.Fatalf("a quantity update on a canceled subscription's item: %s, want it refused as a request (400 or 404)", errOf(u))

+ }

+ if s, err := c.GetSubscription(ctx, subID); err == nil && len(s.Items) == 1 && s.Items[0].Quantity != before {

+ t.Fatalf("the quantity of a canceled subscription moved from %d to %d", before, s.Items[0].Quantity)

}

t.Logf("OBSERVED: a quantity update on a canceled subscription's item is answered %s", errOf(u))

f := raw("POST", itemPath, fenceForm, k("six-fence"))

diff --git a/internal/console/billingquantity.go b/internal/console/billingquantity.go

index 09f3f2f91..d3787302d 100644

--- a/internal/console/billingquantity.go

+++ b/internal/console/billingquantity.go

@@ -103,8 +103,13 @@ const (

// an indeterminate provider error looks like. The operation stays

// outstanding and nothing replaces it. Resolving the review is the way out:

// the worker then retires the operation.

-func billingQuantityUnsettledReason(created int64) string {

- return "billing_quantity_operation_unsettled:" + strconv.FormatInt(created, 10)

+//

+// The review names the operation by its serial, which no two operations of

+// an account share. Nothing else about an operation is unique: two can be

+// created in the same second for the same subscription, and a review of one

+// must never be taken for a decision about the other.

+func billingQuantityUnsettledReason(serial int64) string {

+ return "billing_quantity_operation_unsettled:" + strconv.FormatInt(serial, 10)

}

// prorationBehavior maps the configured policy to the provider's behaviour

@@ -650,14 +655,19 @@ func (s *Store) retireQuantityOperation(accountID int64, key string, now time.Ti

// ran is acknowledged in any way, its idempotency key is consumed at the

// provider (billingstripe.FenceQuantityKey). From then on the provider

// itself refuses the late request, or answers it from memory without

-// running it.

+// running it. There is one exception, and it is a person: an operation

+// whose key cannot be fenced is held with a review, and a resolved review

+// stands in for the fence. No provider key is consumed for it.

// The invariant, enforced by every statement that acknowledges an

// operation (quantityOperationSettled): an operation is acknowledged only

// when the provider answered its own request with success (confirmed_at),

-// or its key is fenced (fenced_at). A decision to retire is made on a

-// provider read taken after the fence, never before it, so a request that

-// ran just before the fence is seen and recorded, not retired as unsent.

+// or its key is fenced (fenced_at). For an operation of the account's

+// current subscription a decision to retire is made on a provider read

+// taken after the fence, never before it, so a request that ran just

+// before the fence is seen and recorded, not retired as unsent. An

+// operation of an ended subscription, or of a subscription the account has

+// left, is acknowledged on the fence alone, with no read.

// Its limit. A consumed key is NOT a permanent fencing token. Stripe

// documents that keys may be removed once they are at least 24 hours old,

@@ -723,7 +733,7 @@ func (s *Store) markQuantityReviewed(op quantityOperation, now time.Time) (bool,

defer tx.Rollback()

var resolved int64

switch err := tx.QueryRow(`SELECT resolved_at FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=?`,

- op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.CreatedAt)).Scan(&resolved); {

+ op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.Serial)).Scan(&resolved); {

case errors.Is(err, sql.ErrNoRows):

return false, nil

case err != nil:

@@ -1065,7 +1075,7 @@ func (s *Server) sendQuantityOperation(ctx context.Context, state billingState,

if merr := s.store.markQuantityFenced(op.AccountID, op.Key, billingstripe.FenceHeldByRequest, quantityClock()); merr != nil {

return quantityStep{}, errors.Join(err, merr)

}

- if rerr := s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.CreatedAt), quantityClock()); rerr != nil {

+ if rerr := s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.Serial), quantityClock()); rerr != nil {

return quantityStep{}, errors.Join(err, rerr)

}

return quantityStep{}, err

@@ -1246,7 +1256,7 @@ func (s *Server) settleQuantityOperation(ctx context.Context, op quantityOperati

if op.FencedAt == 0 {

return false, s.fenceQuantityOperation(ctx, op, now)

}

- return false, s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.CreatedAt), now)

+ return false, s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.Serial), now)

}

// fenceQuantityOperation consumes op's idempotency key at the provider (see

@@ -1260,7 +1270,7 @@ func (s *Server) fenceQuantityOperation(ctx context.Context, op quantityOperatio

if op.settled() || op.SendLeaseUntil > quantityClock().Unix() {

return nil

}

- reason := billingQuantityUnsettledReason(op.CreatedAt)

+ reason := billingQuantityUnsettledReason(op.Serial)

switch rev, err := s.store.billingReview(op.AccountID, op.Subscription, reason); {

case errors.Is(err, sql.ErrNoRows):

case err != nil:

@@ -1277,7 +1287,14 @@ func (s *Server) fenceQuantityOperation(ctx context.Context, op quantityOperatio

kind, err := s.billing.client.FenceQuantityKey(ctx, op.Item, op.Generation, op.Key)

if err != nil {

var refused *billingstripe.Error

- if errors.As(err, &refused) && refused.Code == billingstripe.CodeAPIError && refused.HTTPStatus >= 400 && refused.HTTPStatus < 500 && refused.HTTPStatus != http.StatusTooManyRequests {

+ // Two answers will not change by asking again, and get a review:

+ // a definite refusal, and a server error answered from memory (the

+ // fence request itself began earlier and ended in a result the

+ // provider remembers and calls indeterminate).

+ repeats := errors.As(err, &refused) && refused.Code == billingstripe.CodeAPIError &&

+ ((refused.HTTPStatus >= 400 && refused.HTTPStatus < 500 && refused.HTTPStatus != http.StatusTooManyRequests && refused.HTTPStatus != http.StatusConflict) ||

+ (refused.HTTPStatus >= 500 && refused.Replayed))

+ if repeats {

if rerr := s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, reason, now); rerr != nil {

return errors.Join(err, rerr)

}

diff --git a/internal/console/billingquantity_sim_test.go b/internal/console/billingquantity_sim_test.go

index 2cb6bf1dd..02124f202 100644

--- a/internal/console/billingquantity_sim_test.go

+++ b/internal/console/billingquantity_sim_test.go

@@ -256,6 +256,19 @@ func simulateQuantity(t *testing.T, seed int64) (trace []string, problem string)

logf(" next request is answered 500, remembered, with no effect yet")

}

q.mu.Unlock()

+ // Sometimes a person resolves an operation's open review, which is

+ // the way out for one the worker could not settle.

+ if rng.Intn(5) == 0 {

+ if op, ok, _ := fx.st.quantityOperationFor(OpsAccountID); ok && op.AckedAt == 0 {

+ reason := billingQuantityUnsettledReason(op.Serial)

+ if rev, err := fx.st.billingReview(OpsAccountID, op.Subscription, reason); err == nil && rev.ResolvedAt == 0 {

+ _ = fx.st.SetOperator(reviewOperator, true)

+ if err := fx.st.resolveBillingReview(billingReviewResolution{AccountID: OpsAccountID, Subscription: op.Subscription, Reason: reason, Occurrence: rev.Occurrences, Actor: reviewOperator, Note: "sim"}, clock()); err == nil {

+ logf(" a person resolves the review of operation %d", op.Serial)

+ }

switch rng.Intn(4) {

case 0:

if members < 16 {

@@ -322,14 +335,17 @@ func simulateQuantity(t *testing.T, seed int64) (trace []string, problem string)

q.mu.Lock()

atProvider := q.quantity

q.mu.Unlock()

- // Held by a review a person must resolve is a settled state: the

- // additions are owed, and the account says why they wait.

- for _, r := range reviews {

- if strings.HasSuffix(r, "open=1") {

- return trace, ""

- }

+ // Pending additions are a settled state only when the thing that

+ // holds them says so itself: the outstanding operation has its own

+ // open review (by its serial), or, with no operation outstanding,

+ // the account has an open mismatch review. Some other review that

+ // happens to be open (an earlier operation's dating hold) explains

+ // nothing about this one and is not accepted.

+ if why := quantityHeldBy(op, reviews); why != "" {

+ logf(" settled, held: %s", why)

+ return trace, ""

}

- return trace, fmt.Sprintf("Settles: %d addition(s) still pending with no review; billed %d (status %s, period %d), provider %d, members %d, operation %+v, reviews %v", len(pending), state.Quantity, state.Status, state.PeriodStart, atProvider, members, op, reviews)

+ return trace, fmt.Sprintf("Settles: %d addition(s) still pending with no review of what holds them; billed %d (status %s, period %d), provider %d, members %d, operation %+v, reviews %v", len(pending), state.Quantity, state.Status, state.PeriodStart, atProvider, members, op, reviews)

}

state, err := fx.st.billingAccount(OpsAccountID)

if err != nil {

@@ -346,3 +362,45 @@ func simulateQuantity(t *testing.T, seed int64) (trace []string, problem string)

}

return trace, ""

}

+// quantityHeldBy says which open review accounts for additions that are

+// still pending when a run has settled, or "" when none does. reviews are

+// "<reason> open=<0|1>".

+func quantityHeldBy(op quantityOperation, reviews []string) string {

+ open := map[string]bool{}

+ for _, r := range reviews {

+ if reason, ok := strings.CutSuffix(r, " open=1"); ok {

+ open[reason] = true

+ }

+ if op.Key != "" && op.AckedAt == 0 {

+ if reason := billingQuantityUnsettledReason(op.Serial); open[reason] {

+ return reason

+ }

+ return ""

+ }

+ if open[billingQuantityMismatchReason] {

+ return billingQuantityMismatchReason

+ }

+ return ""

+// The settle judgement is not satisfied by a review that belongs to

+// something else. An outstanding operation with no review of its own is

+// unsettled even when an earlier operation's dating hold is open.

+func TestQuantitySimulationOracleIgnoresUnrelatedReviews(t *testing.T) {

+ stuck := quantityOperation{Key: "k", Serial: 7}

+ unrelated := []string{billingQuantityDateReviewReason(123) + " open=1", billingQuantityUnsettledReason(6) + " open=1", billingQuantityMismatchReason + " open=1"}

+ if why := quantityHeldBy(stuck, unrelated); why != "" {

+ t.Fatalf("an outstanding operation with no review of its own was taken as held by %q", why)

+ }

+ if why := quantityHeldBy(stuck, append(unrelated, billingQuantityUnsettledReason(7)+" open=0")); why != "" {

+ t.Fatalf("a resolved review was taken as holding the operation: %q", why)

+ }

+ if why := quantityHeldBy(stuck, []string{billingQuantityUnsettledReason(7) + " open=1"}); why == "" {

+ t.Fatal("the operation's own open review was not recognised")

+ }

+ if why := quantityHeldBy(quantityOperation{Key: "k", Serial: 7, AckedAt: 1}, []string{billingQuantityDateReviewReason(123) + " open=1"}); why != "" {

+ t.Fatalf("with nothing outstanding, a dating hold was taken as the reason additions are pending: %q", why)

+ }

diff --git a/internal/console/billingquantity_test.go b/internal/console/billingquantity_test.go

index 23512c6e0..8ad1d6d5e 100644

--- a/internal/console/billingquantity_test.go

+++ b/internal/console/billingquantity_test.go

@@ -49,6 +49,8 @@ type quantityStripe struct {

duringPost func()

// refuseFence makes the provider refuse a fence request as invalid.

refuseFence bool

+ // fence500 makes the next fence request end in a remembered 500.

+ fence500 bool

// arriving is the key of the request the hooks are being run for.

arriving string

// duringFence is the same for a fence request.

@@ -121,6 +123,20 @@ func (q *quantityStripe) serve(w http.ResponseWriter, r *http.Request) {

if q.byKey == nil {

q.byKey, q.params = map[string]int64{}, map[string]string{}

}

+ if r.PostForm.Get("quantity") == "" && q.fence500 {

+ // The fence request itself ends in a server error the provider

+ // remembers: every later fence under the key replays it.

+ if q.byKey == nil {

+ q.byKey, q.params = map[string]int64{}, map[string]string{}

+ }

+ if q.saved500 == nil {

+ q.saved500 = map[string]bool{}

+ }

+ q.fence500 = false

+ q.byKey[key], q.params[key], q.saved500[key] = q.quantity, r.PostForm.Encode(), true

+ w.WriteHeader(http.StatusInternalServerError)

+ return

+ }

if r.PostForm.Get("quantity") == "" && q.refuseFence {

w.WriteHeader(http.StatusBadRequest)

_, _ = w.Write([]byte(`{"error":{"type":"invalid_request_error","message":"refused"}}`))

@@ -2044,7 +2060,7 @@ func TestQuantityRefusedFenceHoldsForAPersonAndIsRetiredOnceReviewed(t *testing.

if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.Key != op.Key || cur.AckedAt != 0 || cur.FencedAt != 0 {

t.Fatalf("acknowledged or marked fenced over a refused fence: %+v", cur)

}

- reason := billingQuantityUnsettledReason(op.CreatedAt)

+ reason := billingQuantityUnsettledReason(op.Serial)

rev, err := fx.st.billingReview(OpsAccountID, "sub_team", reason)

if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

t.Fatalf("review for the refused fence: %+v %v", rev, err)

@@ -2116,7 +2132,7 @@ func TestQuantityIndeterminateRequestIsHeldNotRetired(t *testing.T) {

if len(q.posts) != 0 || q.quantity != 10 {

t.Fatalf("something was sent in its place: posts %+v quantity %d", q.posts, q.quantity)

}

- rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(op.CreatedAt))

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(op.Serial))

if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

t.Fatalf("no single open review for the indeterminate operation: %+v %v", rev, err)

}

@@ -2341,8 +2357,124 @@ func TestQuantityReplayedServerErrorHoldsAtOnce(t *testing.T) {

if cur.Key != op.Key || cur.AckedAt != 0 || cur.FenceKind != billingstripe.FenceHeldByRequest {

t.Fatalf("the operation was retired or replaced: %+v", cur)

}

- rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(op.CreatedAt))

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(op.Serial))

if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

t.Fatalf("no single open review: %+v %v", rev, err)

}

+// A resolved review settles the operation it was filed for and no other.

+// The first operation is held, a person resolves its review, and it is

+// retired. A second operation is then recorded in the same second for the

+// same subscription (a clock that returned to it), and it too cannot be

+// fenced. The first one's resolution says nothing about it: it gets a

+// review of its own and is not retired on the strength of the earlier one.

+func TestQuantityResolvedReviewSettlesOnlyItsOwnOperation(t *testing.T) {

+ fx, q, op := agedQuantityFixture(t)

+ ctx := context.Background()

+ aged := time.Now().Add(-quantityOperationReplayLimit - time.Minute).Unix()

+ if _, err := fx.st.db.Exec(`UPDATE billing_quantity_operations SET created_at=? WHERE account_id=?`, aged, OpsAccountID); err != nil {

+ t.Fatal(err)

+ }

+ op, _, _ = fx.st.quantityOperationFor(OpsAccountID)

+ q.mu.Lock()

+ q.refuseFence = true

+ q.mu.Unlock()

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("a refused fence must surface")

+ }

+ first := billingQuantityUnsettledReason(op.Serial)

+ if err := fx.st.SetOperator(reviewOperator, true); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.resolveBillingReview(billingReviewResolution{AccountID: OpsAccountID, Subscription: "sub_team", Reason: first, Occurrence: 1, Actor: reviewOperator, Note: "checked"}, time.Now()); err != nil {

+ t.Fatal(err)

+ }

+ // Two passes: the resolution is recorded on the first operation, and

+ // it is retired. No replacement is sent while the provider refuses.

+ for i := 0; i < 2; i++ {

+ _ = fx.srv.billingQuantityPass(ctx, time.Now().UTC())

+ }

+ // The second operation, forced into the first one's second.

+ second, ok, _ := fx.st.quantityOperationFor(OpsAccountID)

+ if !ok || second.Serial == op.Serial {

+ // The worker has not recorded one yet (its request would also be

+ // refused); record it directly.

+ state, _ := fx.st.billingAccount(OpsAccountID)

+ second = op

+ second.Key, second.Serial, second.AckedAt, second.FencedAt, second.FenceKind, second.Revision = "carloku-qty-second", 0, 0, 0, "", state.Revision

+ second.CreatedAt = time.Now().Unix()

+ if started, err := fx.st.beginQuantityOperation(&second); err != nil || !started {

+ t.Fatalf("fixture: the second operation: started=%v err=%v", started, err)

+ }

+ if _, err := fx.st.db.Exec(`UPDATE billing_quantity_operations SET created_at=?, send_lease_until=0 WHERE account_id=?`, aged, OpsAccountID); err != nil {

+ t.Fatal(err)

+ }

+ second, _, _ = fx.st.quantityOperationFor(OpsAccountID)

+ if second.Serial == op.Serial || second.CreatedAt != op.CreatedAt || second.AckedAt != 0 {

+ t.Fatalf("fixture: want a second outstanding operation in the first one's second: first %+v second %+v", op, second)

+ }

+ for i := 0; i < 3; i++ {

+ _ = fx.srv.billingQuantityPass(ctx, time.Now().UTC())

+ }

+ cur, _, _ := fx.st.quantityOperationFor(OpsAccountID)

+ if cur.Serial != second.Serial || cur.AckedAt != 0 || cur.FenceKind == fenceKindReviewed {

+ t.Fatalf("the second operation was settled by the first one's review: %+v", cur)

+ }

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(second.Serial))

+ if err != nil || rev.ResolvedAt != 0 {

+ t.Fatalf("the second operation has no open review of its own: %+v %v", rev, err)

+ }

+ if old, _ := fx.st.billingReview(OpsAccountID, "sub_team", first); old.ResolvedAt == 0 {

+ t.Fatalf("the first operation's review was reopened: %+v", old)

+ }

+// The fence request itself ends in a server error the provider remembers.

+// Asking again is answered the same way from memory. The operation cannot

+// be settled by the worker, so a person is told: the review is filed at the

+// replay, and the provider is not asked again while it stands open.

+func TestQuantityFenceThatEndsInARememberedServerErrorIsHeldForAPerson(t *testing.T) {

+ fx, q, op := agedQuantityFixture(t)

+ ctx := context.Background()

+ if _, err := fx.st.db.Exec(`UPDATE billing_quantity_operations SET created_at=? WHERE account_id=?`, time.Now().Add(-quantityOperationReplayLimit-time.Minute).Unix(), OpsAccountID); err != nil {

+ t.Fatal(err)

+ }

+ op, _, _ = fx.st.quantityOperationFor(OpsAccountID)

+ q.mu.Lock()

+ q.fence500 = true

+ q.mu.Unlock()

+ fences := 0

+ q.duringFence = func() { fences++ }

+ // The first answer is a plain 500: it may be transient, nothing is filed.

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("the fence's 500 must surface")

+ }

+ reason := billingQuantityUnsettledReason(op.Serial)

+ if rev, err := fx.st.billingReview(OpsAccountID, "sub_team", reason); err == nil && rev.Occurrences > 0 {

+ t.Fatalf("a first, unreplayed 500 filed a review: %+v", rev)

+ }

+ // The second is the same 500 from memory.

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("the replayed 500 must surface")

+ }

+ for i := 0; i < 3; i++ {

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatalf("pass %d with the review open: %v", i, err)

+ }

+ if fences != 2 {

+ t.Fatalf("the provider was asked to fence %d times, want twice (the 500 and its replay)", fences)

+ }

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", reason)

+ if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

+ t.Fatalf("no single open review for the fence that cannot be taken: %+v %v", rev, err)

+ }

+ if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.Key != op.Key || cur.AckedAt != 0 || cur.FencedAt != 0 {

+ t.Fatalf("the operation was acknowledged or marked fenced: %+v", cur)

+ }

+ if len(q.posts) != 0 {

+ t.Fatalf("something was sent: %+v", q.posts)

+ }

diff --git a/internal/objstore/fence_aws_test.go b/internal/objstore/fence_aws_test.go

index f53da8df9..b96a6f734 100644

--- a/internal/objstore/fence_aws_test.go

+++ b/internal/objstore/fence_aws_test.go

@@ -4,7 +4,6 @@ import (

"context"

"errors"

"os"

- "strings"

"testing"

"time"

@@ -27,9 +26,15 @@ import (

// It changes nothing.

// What it establishes: S3 and IAM apply the signing-time window to requests

-// signed by this package's signer, the SDK's retries do not get round it

-// (each attempt is signed again with the same stamp), and the signer signs

-// an S3 key with characters that need escaping the way S3 verifies it.

+// signed by this package's signer, a request past the window is still

+// refused with the SDK's retry policy switched on, and the signer signs an

+// S3 key with characters that need escaping the way S3 verifies it.

+//

+// It does not establish what a retry does: AWS has not been seen to make

+// the SDK retry here, and whether a second attempt was made is logged, not

+// required. That every attempt carries the same stamp and raises the walk's

+// doubt is shown against a local server, in internal/sigfence and in

+// TestProductionClientsCarryTheFenceAndTheAttemptWatcher.

// What it does not: it sends no delete and no write. The window is applied

// to the signature before the operation is looked at, so there is no reason

@@ -115,14 +120,12 @@ func TestFenceAgainstAWS(t *testing.T) {

_, err := s3c.GetObject(withFence(ctx, time.Now().Add(-time.Minute)), &awss3.GetObjectInput{Bucket: aws.String(bucket), Key: aws.String("carlos fence probe/a+b=c~d (1).json")})

answer := code(err)

t.Logf("a key that needs escaping, signed a minute ago: S3 answers %q", answer)

- if answer == "SignatureDoesNotMatch" || strings.HasPrefix(answer, "error:") || answer == "ok" {

+ if answer != "NoSuchKey" && answer != "AccessDenied" {

t.Errorf("a key that needs escaping: %q, want the operation's own refusal (NoSuchKey or AccessDenied)", answer)

}

- // With the SDK's retries on, as the walks run. The SDK treats a skewed

- // request as retryable and signs each attempt again. Every attempt

- // must carry the same old stamp and be refused, and the walk's doubt

- // must be raised by the retry.

+ // With the SDK's retry policy on, as the walks run: the request must

+ // still be refused. Whether the SDK made a second attempt is logged.

retrying, _ := clients(3)

fenced, doubt := sigfence.Track(withFence(ctx, time.Now().Add(-fenceWindow-time.Minute)))

_, err = retrying.ListObjectsV2(fenced, &awss3.ListObjectsV2Input{Bucket: aws.String(bucket), MaxKeys: aws.Int32(1)})

diff --git a/internal/objstore/fence_wiring_test.go b/internal/objstore/fence_wiring_test.go

new file mode 100644

index 000000000..fe8ab87fc

--- /dev/null

+++ b/internal/objstore/fence_wiring_test.go

@@ -0,0 +1,106 @@

+package objstore

+import (

+ "context"

+ "net/http"

+ "net/http/httptest"

+ "sync"

+ "testing"

+ "time"

+ "github.com/carlosframework/platform/internal/release"

+ "github.com/carlosframework/platform/internal/sigfence"

+// The fence is only as good as the clients that carry it. These are the

+// production constructors, not clients built for a test: the provisioner's

+// S3 and IAM clients, and the deployment bucket's store. Each must stamp a

+// request made under a walk's context with the walk's lease instant, stamp

+// any other request with the present, and raise the walk's doubt when a

+// request needed a second attempt. Taking the signer or the attempt watcher

+// off any of them fails here.

+func TestProductionClientsCarryTheFenceAndTheAttemptWatcher(t *testing.T) {

+ var mu sync.Mutex

+ var stamps []string

+ failNext := false

+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

+ mu.Lock()

+ stamps = append(stamps, r.Header.Get("X-Amz-Date"))

+ fail := failNext

+ failNext = false

+ mu.Unlock()

+ if fail {

+ w.WriteHeader(http.StatusInternalServerError)

+ return

+ }

+ w.WriteHeader(http.StatusNotFound)

+ }))

+ defer srv.Close()

+ for k, v := range map[string]string{

+ "AWS_ACCESS_KEY_ID": "AKIDEXAMPLE", "AWS_SECRET_ACCESS_KEY": "secret", "AWS_SESSION_TOKEN": "", "AWS_REGION": "eu-west-1", "AWS_PROFILE": "",

+ "AWS_EC2_METADATA_DISABLED": "true", "AWS_CONFIG_FILE": "/dev/null", "AWS_SHARED_CREDENTIALS_FILE": "/dev/null",

+ "CARLOS_S3_ENDPOINT": srv.URL, "AWS_ENDPOINT_URL_IAM": srv.URL,

+ } {

+ t.Setenv(k, v)

+ }

+ ctx := context.Background()

+ store, err := release.NewS3Store(ctx, "deployment-bucket")

+ if err != nil {

+ t.Fatal(err)

+ }

+ prov, err := NewAWSProvisioner(ctx, "eu-west-1", srv.URL, "")

+ if err != nil {

+ t.Fatal(err)

+ }

+ leased := time.Now().Add(-10 * time.Minute).UTC().Truncate(time.Second)

+ want := leased.Format("20060102T150405Z")

+ last := func() string {

+ mu.Lock()

+ defer mu.Unlock()

+ if len(stamps) == 0 {

+ return ""

+ }

+ return stamps[len(stamps)-1]

+ }

+ calls := map[string]func(context.Context){

+ "the deployment bucket's store (GetObject)": func(c context.Context) { _, _, _ = store.Get(c, "apps/bab/titogo/store.json") },

+ "the provisioner's S3 client (HeadBucket)": func(c context.Context) { _, _ = prov.(BucketProber).BucketExists(c, "carlos-store-x") },

+ "the provisioner's IAM client (DeleteKey)": func(c context.Context) { _ = prov.DeleteKey(c, "carlos-store-bab-titogo", "AKIAEXAMPLEKEY") },

+ }

+ for name, call := range calls {

+ fenced, doubt := sigfence.Track(withFence(ctx, leased))

+ call(fenced)

+ if got := last(); got != want {

+ t.Errorf("%s under a walk's context was stamped %q, want the lease instant %q", name, got, want)

+ }

+ if doubt.Raised() {

+ t.Errorf("%s: one attempt raised doubt", name)

+ }

+ call(ctx)

+ if got := last(); got == want || got == "" {

+ t.Errorf("%s outside any walk was stamped %q: want the present", name, got)

+ }

+ // A first attempt that fails in a way the SDK retries.

+ mu.Lock()

+ failNext = true

+ before := len(stamps)

+ mu.Unlock()

+ fenced, doubt = sigfence.Track(withFence(ctx, leased))

+ call(fenced)

+ mu.Lock()

+ attempts := append([]string(nil), stamps[before:]...)

+ mu.Unlock()

+ if len(attempts) < 2 {

+ t.Errorf("%s: fixture: the SDK made %d attempt(s), want a retry", name, len(attempts))

+ continue

+ }

+ for _, got := range attempts {

+ if got != want {

+ t.Errorf("%s: a retried attempt was stamped %q, want the lease instant %q", name, got, want)

+ }

+ if !doubt.Raised() {

+ t.Errorf("%s: a request that needed a second attempt left no doubt on its walk", name)

+ }

diff --git a/internal/objstore/ownership_test.go b/internal/objstore/ownership_test.go

index 13d2b72b4..b86ac4447 100644

--- a/internal/objstore/ownership_test.go

+++ b/internal/objstore/ownership_test.go

@@ -2103,3 +2103,131 @@ func TestTeardownDoesNotDeleteASidecarWrittenAfterItLooked(t *testing.T) {

t.Fatal("the record was deleted by a teardown that did not finish")

}

+// A record whose bucket name this build would refuse (an older build's, or

+// one edited by hand), and a record that names no bucket, still name a real

+// user and real keys. A name that fails validation is never a reason to act

+// without a lease: such a store is leased under a key of its own, and its

+// rotations and its teardown exclude each other like any other walk's.

+func TestStoreWithAnUnsupportedBucketNameIsStillLeased(t *testing.T) {

+ for _, bucket := range []string{"Legacy_Bucket.With/Slash", ""} {

+ ctx := context.Background()

+ s := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ leaseClock(t)

+ user := UserName("bab", "titogo")

+ if _, err := p.EnsureUser(ctx, user, "/carlos/", "arn:aws:s3:::carlos-store-somewhere", nil); err != nil {

+ t.Fatal(err)

+ }

+ keyID, _, err := p.MintKey(ctx, user)

+ if err != nil {

+ t.Fatal(err)

+ }

+ rec := &Record{Declared: &Declared{EnvPrefix: "TITO_S3", At: t0, By: "member@example.com"},

+ Grant: &GrantBlock{Backend: "aws", Bucket: bucket, Source: GrantSourceOperator, At: t0, By: "op"},

+ Provisioned: &Provisioned{Bucket: bucket, Region: "eu-west-1", IAMUser: user, KeyID: keyID, At: t0}}

+ if bucket == "" {

+ // The crashed-grant shape: a grant block and nothing provisioned.

+ rec.Provisioned = nil

+ }

+ if _, err := Put(ctx, s, "bab", "titogo", rec, ""); err != nil {

+ t.Fatal(err)

+ }

+ if ValidBucketName(bucket) {

+ t.Fatalf("fixture: %q is a valid bucket name", bucket)

+ }

+ // Another walk on the same store is in progress.

+ other, err := acquireBucketWalk(ctx, s, bucket, "bab", "titogo", "rotate")

+ if err != nil {

+ t.Fatalf("bucket %q: the lease for an unsupported name: %v", bucket, err)

+ }

+ if rec.Provisioned != nil {

+ if _, err := Rotate(ctx, s, p, pub, "bab", "titogo", t0.Add(time.Minute)); !errors.Is(err, ErrBucketBusy) {

+ t.Fatalf("bucket %q: a rotation ran beside another walk on the same store: %v", bucket, err)

+ }

+ if _, err := RotateFinish(ctx, s, p, "bab", "titogo"); !errors.Is(err, ErrBucketBusy) && !errors.Is(err, ErrNoPendingFinish) {

+ t.Fatalf("bucket %q: a rotation finish ran beside another walk: %v", bucket, err)

+ }

+ if err := Teardown(ctx, s, p, nil, "bab", "titogo"); !errors.Is(err, ErrBucketBusy) {

+ t.Fatalf("bucket %q: a teardown ran beside another walk on the same store: %v", bucket, err)

+ }

+ if u, ok := p.Users[user]; !ok || len(u.Keys) != 1 || u.Keys[keyID] == "" {

+ t.Fatalf("bucket %q: a refused walk touched the user's keys: %+v", bucket, u)

+ }

+ // A store of another app, or with another name, is not held up.

+ if w, err := acquireBucketWalk(ctx, s, bucket, "bab", "second", "rotate"); bucket == "" && err != nil {

+ t.Fatalf("an app-keyed lease held up another app: %v", err)

+ } else if w != nil {

+ w.release(true)

+ }

+ other.release(true)

+ if err := Teardown(ctx, s, p, nil, "bab", "titogo"); err != nil {

+ t.Fatalf("bucket %q: the teardown once the name was free: %v", bucket, err)

+ }

+ if _, ok := p.Users[user]; ok {

+ t.Fatalf("bucket %q: the teardown left the user", bucket)

+ }

+// A teardown request reads its store and is held before it takes the lease.

+// Another process finishes the teardown. The member creates the store

+// again: the grant mints the same name, delivers a new key, and data is

+// written. The first request then wakes, takes the lease (it is free) and

+// reads the record again. The name is the same. The estate is not: it is a

+// store that did not exist when the request was made. The teardown stops

+// before it writes or calls anything.

+func TestStaleTeardownDoesNotDestroyAStoreRecreatedUnderTheSameName(t *testing.T) {

+ 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")

+ first, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0)

+ if err != nil {

+ t.Fatal(err)

+ }

+ var again *Record

+ s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)}

+ s.hook = func() {

+ if err := Teardown(ctx, dir, p, nil, "bab", "titogo"); err != nil {

+ t.Errorf("the teardown that finishes first: %v", err)

+ }

+ advance(time.Hour)

+ declare(t, dir, "bab", "titogo", "TITO_S3")

+ var err error

+ if again, err = Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); err != nil {

+ t.Errorf("the store created again: %v", err)

+ return

+ }

+ p.Buckets[ledgerBucket]["invoice.pdf"] = []byte("the new store's data")

+ }

+ err = Teardown(ctx, s, p, nil, "bab", "titogo")

+ if !s.done || again == nil {

+ t.Fatal("fixture: the teardown never reached its lease")

+ }

+ if again.Provisioned.Bucket != first.Provisioned.Bucket || again.Provisioned.KeyID == first.Provisioned.KeyID {

+ t.Fatalf("fixture: want the same name and a new key: %+v then %+v", first.Provisioned, again.Provisioned)

+ }

+ if !errors.Is(err, release.ErrCASConflict) {

+ t.Fatalf("a teardown made against the store before this one: %v, want it to stop", err)

+ }

+ if got := string(p.Buckets[ledgerBucket]["invoice.pdf"]); got != "the new store's data" {

+ t.Fatalf("the new store's data was destroyed by a request made against its predecessor (object now %q)", got)

+ }

+ u, ok := p.Users[UserName("bab", "titogo")]

+ if !ok || u.Keys[again.Provisioned.KeyID] == "" {

+ t.Fatalf("the new store's user or key was removed: %+v", u)

+ }

+ rec, _, gerr := Get(ctx, dir, "bab", "titogo")

+ if gerr != nil || rec == nil || rec.Teardown != nil || rec.Provisioned == nil || rec.Provisioned.KeyID != again.Provisioned.KeyID {

+ t.Fatalf("the new store's record was written to: %+v %v", rec, gerr)

+ }

+ ledger, _, _ := ReadBucketOwnership(ctx, dir, ledgerBucket)

+ if ledger == nil || ledger.Destroying != nil {

+ t.Fatalf("the stale teardown marked the new store's bucket as being destroyed: %+v", ledger)

+ }

diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go

index c07195ae5..f21e8c328 100644

--- a/internal/objstore/walk.go

+++ b/internal/objstore/walk.go

@@ -636,15 +636,6 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app

// 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

- if !ValidBucketName(bucket) {

- // A name no grant of this build would have written has no lease to

- // take. The rotation runs as it always did.

- rec, etag, err := Get(ctx, s, acct, app)

- if err == nil && (rec == nil || rec.Provisioned == nil) {

- err = ErrNotProvisioned

- }

- return ctx, rec, etag, func(error) {}, err

- }

walk, err := acquireBucketWalk(ctx, s, bucket, acct, app, "rotate")

if err != nil {

return ctx, nil, "", nil, err

@@ -737,7 +728,17 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

case rec.Grant != nil:

leased = rec.Grant.Bucket

}

- if leased != "" && ValidBucketName(leased) {

+ // The estate this teardown was asked to destroy, as first read: which

+ // grant, which key, and whether a teardown had already begun on it. A

+ // bucket name is not an estate. The minted name is the same for every

+ // store an app ever has, so a store torn down and created again while

+ // this request waited has the same name and is not the store the

+ // request was made against.

+ asked := estateOf(rec)

+ // Always: a record whose bucket name this build would refuse, or that

+ // names no bucket, still names a user and keys, and is leased under a

+ // key of its own (walkLeaseKey).

+ {

walk, werr := acquireBucketWalk(ctx, s, leased, acct, app, "teardown")

if werr != nil {

return werr

@@ -772,9 +773,13 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

case rec.Grant != nil:

now = rec.Grant.Bucket

}

- if now != "" && now != leased {

+ if now != leased {

return fmt.Errorf("objstore: store record for %s/%s changed while its teardown was starting (it named %s and now names %s); run it again: %w", acct, app, leased, now, release.ErrCASConflict)

}

+ // And it must be the same estate, not only the same name.

+ if estateOf(rec) != asked {

+ return fmt.Errorf("objstore: the store of %s/%s was replaced while its teardown was starting; the request was made against the store before it, and nothing was touched: %w", acct, app, release.ErrCASConflict)

+ }

if rec.Grant == nil && rec.Provisioned == nil {

return deleteRecordIfUnchanged(ctx, s, acct, app, startKeyID, startBucket)

}

@@ -1049,6 +1054,29 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

return deleteRecordIfUnchanged(ctx, s, acct, app, startKeyID, startBucket)

}

+// storeEstate identifies the estate a record describes, as far as a walk

+// that waited for a lease needs to recognise it again.

+type storeEstate struct {

+ bucket, keyID string

+ grantedAt int64

+ tearingDown bool

+func estateOf(rec *Record) storeEstate {

+ var e storeEstate

+ if rec == nil {

+ return e

+ }

+ if rec.Grant != nil {

+ e.bucket, e.grantedAt = rec.Grant.Bucket, rec.Grant.At.UnixNano()

+ }

+ if rec.Provisioned != nil {

+ e.bucket, e.keyID = rec.Provisioned.Bucket, rec.Provisioned.KeyID

+ }

+ e.tearingDown = rec.Teardown != nil

+ return e

// walkEndedClean reports whether a walk that returned err left nothing on

// its way to the backend: it succeeded, or it was refused before it called

// the backend, or it stopped on a decision (a retained bucket) after every

diff --git a/internal/objstore/walk_sim_test.go b/internal/objstore/walk_sim_test.go

index e4c584af7..fe45351c3 100644

--- a/internal/objstore/walk_sim_test.go

+++ b/internal/objstore/walk_sim_test.go

@@ -64,6 +64,10 @@ type simApp struct {

// or teardown stopped part-way. "retained": its teardown revoked its

// access and reported that the bucket was left to another account.

state string

+ // walks counts the walks started for this app, so that a walk which

+ // was refused before it began can tell whether another walk of the

+ // same app ran while it waited and left a state of its own.

+ walks int

}

func (a *simApp) name() string { return a.acct + "/" + a.app }

@@ -114,12 +118,18 @@ func (p *interleavingProvisioner) DeleteUser(ctx context.Context, user string) e

type simStore struct {

release.Store

failPut string // "", "before" or "after"

+ // beforeLease runs when a walk reads the lease object it is about to

+ // take: after its first read of the record, before it holds anything.

+ beforeLease func()

}

func (s *simStore) Get(ctx context.Context, key string) ([]byte, string, error) {

if err := memFenced(ctx); err != nil {

return nil, "", err

}

+ if hook := s.beforeLease; hook != nil && strings.HasPrefix(key, "objstore/walks/") {

+ hook()

+ }

return s.Store.Get(ctx, key)

}

@@ -194,6 +204,25 @@ func TestBucketNameInvariantsUnderRandomWalks(t *testing.T) {

// of one mid-walk check and repair it. Only the oracle's own test sets it.

var simTransientFault bool

+// simRegrantFault makes the simulation behave as if a second grant of a

+// live store had lost the store's object. Only the oracle's own test sets

+// it: the model writes the object after every successful grant, and must

+// look for it first.

+var simRegrantFault bool

+// A second grant of a live store that loses the store's data is reported,

+// not papered over by the model writing the object again.

+func TestSimulationOracleSeesARegrantThatLosesData(t *testing.T) {

+ simRegrantFault = true

+ defer func() { simRegrantFault = false }()

+ for seed := int64(0); seed < 200; seed++ {

+ if _, problem := simulateBucketName(t, seed, 40); strings.Contains(problem, "was granted again, and its object is gone") {

+ return

+ }

+ t.Fatal("200 seeds with a lossy re-grant injected and the oracle reported none of them")

// The mid-walk check is real: a violation that exists only between a nested

// walk's return and the outer walk's next step is reported, not repaired

// out of sight.

@@ -236,10 +265,20 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

declare(t, s, a.acct, a.app, "S3")

}

was := a.state

+ a.walks++

+ mine := a.walks

_, err := Grant(ctx, walked, p, testDeployment(), pub, a.acct, a.app, "op", GrantSourceOperator, a.adopt, at)

logf("grant %s: %v", a.name(), err)

- if errors.Is(err, ErrBucketBusy) {

- // Refused before it began. The store is what it was.

+ if errors.Is(err, ErrBucketBusy) || errors.Is(err, release.ErrCASConflict) && a.walks != mine {

+ // Refused before it began (another walk holds the name, or the

+ // store was replaced while this one waited for its lease). The

+ // store is what the walks that did run left it.

+ return

+ }

+ if errors.Is(err, ErrFenced) && a.walks != mine {

+ // Frozen, taken over, and refused by the fence on waking. A

+ // walk of this same app ran in the middle and left the store

+ // in a state of its own; the frozen walk did nothing after it.

return

}

if err != nil {

@@ -250,6 +289,13 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

}

return

}

+ // A store that was already live is owed its data across a second

+ // grant. It is looked for before the model writes it again.

+ if was == "live" {

+ if b, ok := mem.Buckets[ledgerBucket]; !ok || string(b[object(a)]) != a.name() || simRegrantFault {

+ mintLeak = fmt.Sprintf("Data: %s was live, was granted again, and its object is gone", a.name())

+ }

a.state = "live"

// An app an operator adopted the name for while it is held (and

// which is live now) is sharing by the operator's choice. Objects

@@ -282,17 +328,34 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

return nil

}

// Asked to go: from this instant the store is not owed its data.

+ // A retained store stays retained while its teardown runs: the

+ // report that its bucket was left to another account stands until

+ // a teardown finishes, and other walks that run in the middle of

+ // this one must see it that way. (Seed 3506 found the model marking

+ // it broken for the length of the walk.)

was := a.state

- a.state = "broken"

+ a.walks++

+ mine := a.walks

+ if was != "retained" {

+ a.state = "broken"

+ }

estate := rec.Grant != nil || rec.Provisioned != nil

err := Teardown(ctx, walked, p, sc, a.acct, a.app)

logf("teardown %s: %v", a.name(), err)

- if errors.Is(err, ErrBucketBusy) {

- // Refused before it began: another walk holds the name. The

- // walk made no call and wrote nothing, so the store is what it

- // was. (Seed 778 found the model forgetting this.)

- a.state = was

- return err

+ if errors.Is(err, ErrBucketBusy) || errors.Is(err, release.ErrCASConflict) || errors.Is(err, ErrFenced) {

+ // Refused before it began (another walk holds the name, or the

+ // store was replaced while this one waited for its lease), or

+ // frozen and refused by the fence on waking. If no other walk

+ // of this app ran meanwhile, a refusal before the start leaves

+ // the store what it was (seed 778 found the model forgetting

+ // this). If one did, the store is what that walk left.

+ if a.walks != mine {

+ return err

+ }

+ if !errors.Is(err, ErrFenced) {

+ a.state = was

+ return err

+ }

}

if err != nil && was == "retained" {

// A retained bucket was reported to a person. A later attempt

@@ -488,12 +551,40 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

logf(" -- back to %s", a.name())

inner = false

}

+ // And sometimes the walk is held between its first read of the

+ // record and its lease, for a long time, while the same app is torn

+ // down and granted again by somebody else. The walk then takes a

+ // lease that is free and reads a record with the same name on it.

+ // It was asked about the store before this one.

+ walked.beforeLease = nil

+ if rng.Intn(6) == 0 {

+ walked.beforeLease = func() {

+ walked.beforeLease = nil

+ if inner {

+ return

+ }

+ inner = true

+ logf(" -- %s's walk is held before its lease; the store is torn down and granted again", a.name())

+ at = at.Add(time.Hour)

+ if teardown(a) == nil {

+ at = at.Add(time.Hour)

+ grant(a, at)

+ }

+ midWalk = a

+ if problem := check(); problem != "" && midProblem == "" {

+ midProblem = fmt.Sprintf("while %s's walk was held before its lease: %s", a.name(), problem)

+ }

+ midWalk = nil

+ logf(" -- back to %s", a.name())

+ inner = false

+ }

if rng.Intn(2) == 0 {

grant(a, at)

} else {

_ = teardown(a)

}

- ip.between = func(string) {}

+ ip.between, walked.beforeLease = func(string) {}, nil

if os.Getenv("CARLOS_SIM_SEED") != "" {

// Reproducing one seed: the state after every step.

var states, objects []string

diff --git a/internal/objstore/walklock.go b/internal/objstore/walklock.go

index ef8ed523d..7d1d100ce 100644

--- a/internal/objstore/walklock.go

+++ b/internal/objstore/walklock.go

@@ -3,6 +3,7 @@ package objstore

import (

"context"

"crypto/rand"

+ "crypto/sha256"

"encoding/hex"

"encoding/json"

"errors"

@@ -70,6 +71,26 @@ var walkClock = time.Now

// evidence, and nothing that reads ledgers should ever meet it.

func walkLockKey(bucket string) string { return "objstore/walks/" + bucket + ".json" }

+// walkLeaseKey is the lease object for whatever a walk is about to act on.

+// Every walk that spends backend authority has one, whatever its record

+// looks like. A bucket name Grant would write is leased under itself. A

+// name it would refuse (a record from an older build, or one edited by

+// hand) cannot be put in an object key as it is, so it is leased under a

+// hash of itself. A record with no bucket name at all is leased under a

+// hash of its app. A name that fails validation is never a reason to run

+// without a lease.

+func walkLeaseKey(bucket, acct, app string) string {

+ if ValidBucketName(bucket) {

+ return walkLockKey(bucket)

+ }

+ what := "app\x00" + acct + "/" + app

+ if bucket != "" {

+ what = "bucket\x00" + bucket

+ }

+ sum := sha256.Sum256([]byte(what))

+ return "objstore/walks/x-" + hex.EncodeToString(sum[:20]) + ".json"

type walkLock struct {

V int `json:"v"`

Bucket string `json:"bucket"`

@@ -111,7 +132,7 @@ func acquireBucketWalk(ctx context.Context, s release.Store, bucket, acct, app,

if _, err := rand.Read(raw[:]); err != nil {

return nil, err

}

- key := walkLockKey(bucket)

+ key := walkLeaseKey(bucket, acct, app)

for attempt := 0; attempt < 3; attempt++ {

now := walkClock()

var carry time.Time

diff --git a/internal/release/trash.go b/internal/release/trash.go

index b802099de..b4bd4a671 100644

--- a/internal/release/trash.go

+++ b/internal/release/trash.go

@@ -209,21 +209,20 @@ func PurgeApp(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app str

// Unreadable — transport error or bytes that will not parse — refuses too,

// the same direction as the production flag: "I could not ask" is not "no

// store". Absent is fine; most apps never declare one.

- if raw, _, err := s.Get(ctx, prefix+"store.json"); err == nil {

- var rec struct {

- Grant *json.RawMessage `json:"grant"`

- Provisioned *json.RawMessage `json:"provisioned"`

- }

- if jerr := json.Unmarshal(raw, &rec); jerr != nil {

- return fmt.Errorf("release: %s/%s: unparseable %sstore.json — refusing to purge: %w: %v",

- acct, app, prefix, ErrStoreProvisioned, jerr)

- }

- if rec.Provisioned != nil || rec.Grant != nil {

- return fmt.Errorf("%s/%s: %w", acct, app, ErrStoreProvisioned)

- }

- } else if !errors.Is(err, os.ErrNotExist) {

- return fmt.Errorf("release: %s/%s: cannot read %sstore.json — refusing to purge: %w: %v",

- acct, app, prefix, ErrStoreProvisioned, err)

+ // The store record is the one object under this prefix that is never

+ // swept unconditionally. The guard above is a read, and a grant admitted

+ // before the app was trashed can land after it: it would create a bucket

+ // and credentials and record them in this very object, and an

+ // unconditional sweep would then delete the only record of a live

+ // estate. So the declaration is deleted here, first, as the generation

+ // the guard just read (purgeStoreDeclaration). A grant that lands

+ // before that changes the generation and the purge refuses. A grant

+ // that lands after it finds no declaration, or loses its write to a

+ // record that is gone. The same rule is applied to the key if the

+ // listing shows it again, and once more before the marker goes.

+ storeKey := prefix + "store.json"

+ if err := purgeStoreDeclaration(ctx, s, acct, app, storeKey); err != nil {

+ return err

}

marker := AppTombstoneKey(acct, app)

keys, err := s.List(ctx, prefix)

@@ -234,9 +233,56 @@ func PurgeApp(ctx context.Context, s Store, pub ed25519.PublicKey, acct, app str

if key == marker {

continue // last, below

}

+ if key == storeKey {

+ if err := purgeStoreDeclaration(ctx, s, acct, app, storeKey); err != nil {

+ return err

+ }

+ continue

+ }

if err := s.Delete(ctx, key); err != nil {

return fmt.Errorf("release: purge %s: %w", key, err)

}

+ if err := purgeStoreDeclaration(ctx, s, acct, app, storeKey); err != nil {

+ return err

+ }

return s.Delete(ctx, marker)

}

+// purgeStoreDeclaration removes an app's store record during a purge, and

+// only if it is a bare declaration: no grant, nothing provisioned. It reads

+// the record and deletes exactly the generation it read. A record that

+// carries estate, cannot be read or parsed, or changes between the read and

+// the delete refuses the purge with ErrStoreProvisioned, in the direction

+// of keeping data: "I could not ask" is not "no store". An absent record

+// is fine; most apps never declare one.

+//

+// Parsed minimally IN PLACE: release cannot import objstore (objstore

+// imports release), so this file is on store.json's parser list,

+// deliberately. BOTH blocks count: `provisioned` is the finished store, and

+// `grant` on its own is a grant that crashed after creating the bucket and

+// the user but before recording them. objstore/walk_test.go pins the

+// agreement between this struct and objstore.Record.

+func purgeStoreDeclaration(ctx context.Context, s Store, acct, app, key string) error {

+ raw, etag, err := s.Get(ctx, key)

+ if errors.Is(err, os.ErrNotExist) {

+ return nil

+ }

+ if err != nil {

+ return fmt.Errorf("release: %s/%s: cannot read %s — refusing to purge: %w: %v", acct, app, key, ErrStoreProvisioned, err)

+ }

+ var rec struct {

+ Grant *json.RawMessage `json:"grant"`

+ Provisioned *json.RawMessage `json:"provisioned"`

+ }

+ if jerr := json.Unmarshal(raw, &rec); jerr != nil {

+ return fmt.Errorf("release: %s/%s: unparseable %s — refusing to purge: %w: %v", acct, app, key, ErrStoreProvisioned, jerr)

+ }

+ if rec.Provisioned != nil || rec.Grant != nil {

+ return fmt.Errorf("%s/%s: %w", acct, app, ErrStoreProvisioned)

+ }

+ if err := DeleteIfMatch(ctx, s, key, etag); err != nil {

+ return fmt.Errorf("release: %s/%s: %s changed or could not be deleted as read — refusing to purge: %w: %v", acct, app, key, ErrStoreProvisioned, err)

+ }

+ return nil

diff --git a/internal/release/trash_test.go b/internal/release/trash_test.go

index 43794d7b3..9638f140b 100644

--- a/internal/release/trash_test.go

+++ b/internal/release/trash_test.go

@@ -469,3 +469,97 @@ func TestPurgeAppRefusesUnreadableStoreRecord(t *testing.T) {

t.Fatalf("a refused purge destroyed objects: %v", got)

}

+// landingStore lets a write land at one store call of a purge, once: the

+// grant that was admitted before the app was trashed and arrives now.

+type landingStore struct {

+ Store

+ at string // "get:<key>", "delete:<key>" or "cdelete:<key>"

+ land func()

+ done bool

+func (l *landingStore) fire(what string) {

+ if !l.done && what == l.at {

+ l.done = true

+ l.land()

+ }

+func (l *landingStore) Get(ctx context.Context, key string) ([]byte, string, error) {

+ body, etag, err := l.Store.Get(ctx, key)

+ l.fire("get:" + key)

+ return body, etag, err

+func (l *landingStore) Delete(ctx context.Context, key string) error {

+ l.fire("delete:" + key)

+ return l.Store.Delete(ctx, key)

+func (l *landingStore) DeleteIfMatch(ctx context.Context, key, etag string) error {

+ l.fire("cdelete:" + key)

+ return DeleteIfMatch(ctx, l.Store, key, etag)

+// A grant admitted while the app was live lands during the purge and

+// records a bucket and credentials in store.json. Wherever it lands, the

+// purge does not delete that record: the declaration is only ever deleted

+// as the generation the purge's own guard read, and a record that carries

+// estate refuses the purge. The estate keeps the one object that names it.

+func TestPurgeAppNeverSweepsAStoreRecordGrantedDuringThePurge(t *testing.T) {

+ _, pub := testKeys(t)

+ const declaration = `{"schema":1,"app":"derp","declared":{"env_prefix":"CARLOS_STORE"}}`

+ const granted = `{"schema":1,"app":"derp","declared":{"env_prefix":"CARLOS_STORE"},"grant":{"backend":"aws","bucket":"carlos-store-tito-bab-derp"},"provisioned":{"bucket":"carlos-store-tito-bab-derp","key_id":"AKIA"}}`

+ storeKey := appPrefix("bab", "derp") + "store.json"

+ for _, tc := range []struct {

+ name, at string

+ // redeclared: the declaration was already deleted by the purge, and

+ // a declare and a grant admitted earlier put a record back.

+ redeclared bool

+ }{

+ {"between the guard's read and its delete", "get:" + storeKey, false},

+ {"as the declaration is being deleted", "cdelete:" + storeKey, false},

+ {"during the sweep, after the declaration was deleted", "delete:" + appPrefix("bab", "derp") + "config.json", true},

+ {"as the marker is about to go", "delete:" + AppTombstoneKey("bab", "derp"), true},

+ } {

+ t.Run(tc.name, func(t *testing.T) {

+ dir := NewDirStore(t.TempDir())

+ seedApp(t, dir, "bab", "derp")

+ if _, err := dir.Put(ctx, appPrefix("bab", "derp")+"config.json", []byte(`{}`), ""); err != nil && !errors.Is(err, ErrCASConflict) {

+ t.Fatal(err)

+ }

+ storeJSON(t, dir, "bab", "derp", declaration)

+ if err := TrashApp(ctx, dir, "bab", "derp", "paul@keymail.dev", time.Now().Add(-31*24*time.Hour)); err != nil {

+ t.Fatal(err)

+ }

+ s := &landingStore{Store: dir, at: tc.at}

+ s.land = func() {

+ _, etag, err := dir.Get(ctx, storeKey)

+ if tc.redeclared {

+ if !errors.Is(err, os.ErrNotExist) {

+ t.Errorf("fixture: the declaration was expected to be gone by now: %v", err)

+ }

+ etag = ""

+ } else if err != nil {

+ t.Error(err)

+ return

+ }

+ if _, err := dir.Put(ctx, storeKey, []byte(granted), etag); err != nil {

+ t.Errorf("the grant that lands: %v", err)

+ }

+ err := PurgeApp(ctx, s, pub, "bab", "derp", time.Now())

+ if !s.done {

+ t.Fatalf("fixture: the purge never reached %s (err %v)", tc.at, err)

+ }

+ raw, _, gerr := dir.Get(ctx, storeKey)

+ if gerr != nil || string(raw) != granted {

+ t.Fatalf("the purge deleted the record of a store granted during it: %q %v (purge: %v)", raw, gerr, err)

+ }

+ if tc.at != "delete:"+AppTombstoneKey("bab", "derp") && !errors.Is(err, ErrStoreProvisioned) {

+ t.Fatalf("PurgeApp = %v, want ErrStoreProvisioned", err)

+ }

+ })

+ }