Skip to content

feat(api): require execute:purchases to arm ladder execution - #847

Merged
cristim merged 1 commit into
mainfrom
feat/ladder-arming-grants
Oct 13, 2026
Merged

cristim merged 1 commit into
mainfrom
feat/ladder-arming-grants

Conversation

@cristim

@cristim cristim commented Oct 13, 2026

Copy link
Copy Markdown
Member

Refs #118

Inert: nothing executes ladder tranches yet. This only decides who may arm the two switches the executor will read.

What changes

  • PUT /api/config: ladder_execution_enabled off to on requires execute:purchases (real user, explicit membership). Checked inside the UpdateGlobalConfigAtomic closure against existing, chained after the RI-exchange and auto-purchase gates. admin:* alone and the admin API key get 403.
  • PUT /api/ladder-config: armed means enabled and mode=auto_approve. Arming, or raising or clearing max_hourly_commit_per_run on an armed config, needs the same grant. Reuses requireAutoPurchaseGrant / credentialKind from feat(api): gate auto-purchase arming behind execute:purchases #841.
  • Not gated: plain laddering_enabled (planning only), switching off, disarming, email_approval, lowering the cap. These work with the admin API key.
  • Cap is NUMERIC(20,6) hourly commitment, so it is validated as exact micro-dollars (LadderCapMicros): more than 6 decimals, below 0.000001 or above 1e12 is a 400, and the gate compares exact micros (no float trap).
  • Migration 000116 (next free number on origin/main at push time): nullable ladder_execution_changed_by/at/via on global_config, read and upserted by the store so other whole-config writes keep them. json:"-", so a PUT body cannot forge them.
  • Structured log on every authorized ladder_config escalation and every switch change (actor, credential kind, config id, old to new).

Known limits

Verification

  • Go unit tests (internal/api, internal/config) green; golangci-lint clean.
  • Mock/handler-level evidence: each gate refuses without the grant, with admin:* only and with the admin API key; allowed with explicit execute:purchases; lowering and disarming allowed with the admin key.
  • Real PG17 (throwaway cluster): fresh migrate leaves both switches OFF with NULL stamps, a whole-config write keeps the stamps, stored micros equal compared micros for 8 caps including the boundaries.
  • Mutants, each fails tests: remove switch gate call, remove ladder_config gate call, > to >=, clear-not-a-raise, off-to-on check dropped, admin-key refusal dropped, stamp dropped, armed ignores mode.

🤖 Generated with Claude Code

Gate the two arming writers for the ladder executor, which nothing runs yet:
PUT /api/config turning ladder_execution_enabled off->on, and PUT
/api/ladder-config arming a config (enabled + auto_approve) or raising or
clearing max_hourly_commit_per_run. Both need a real user holding
execute:purchases; the admin API key and admin:* alone are refused. Disarming,
lowering the cap and plain laddering_enabled stay ordinary updates.

Stamp who/when/via for the execution switch (migration 000116, nullable,
round-tripped by the whole-config upsert). Cap is validated as exact
micro-dollars for NUMERIC(20,6): more than 6 decimals, below 0.000001 or above
1e12 is a 400. Structured log on every authorized escalation and switch change.

Refs #118

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim cristim added urgency/this-sprint Within the current sprint triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm impact/few Limited audience effort/m Days type/bug Defect labels Oct 13, 2026
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 15 billable files and costs up to $3.75.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 24 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available. Your 58 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 9e232310-34b1-4ef6-ae7b-604bcb52179f
📥 Commits

Reviewing files that changed from the base of the PR and between c997ce6 and b6d69a0.

📒 Files selected for processing (15)
  • internal/api/handler_config.go
  • internal/api/handler_ladder.go
  • internal/api/handler_ladder_test.go
  • internal/api/ladder_arming_grant.go
  • internal/api/ladder_arming_grant_test.go
  • internal/config/ladder_cap.go
  • internal/config/store_postgres.go
  • internal/config/store_postgres_coverage_test.go
  • internal/config/store_postgres_ladder_arming_integration_test.go
  • internal/config/store_postgres_pgxmock_test.go
  • internal/config/types.go
  • internal/config/validation.go
  • internal/config/validation_test.go
  • internal/database/postgres/migrations/000116_ladder_execution_switch_stamps.down.sql
  • internal/database/postgres/migrations/000116_ladder_execution_switch_stamps.up.sql
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

Gate verdict: APPROVE at b6d69a0

Independent adversarial review (Opus 5.5, claude-opus-5-5; did not write this PR). Auth + money path: who may arm ladder execution.

Writers (no bypass found). Every writer of global_config.ladder_execution_enabled and ladder_configs at this SHA: PUT /api/config (handler_config.go:98, gated inside the UpdateGlobalConfigAtomic closure, ladderBefore taken from existing before the body unmarshal, chained after the RI-exchange and auto-purchase gates); PUT /api/ladder/configs (handler_ladder.go:131, only ladder_config writer; same (cloud_account_id, provider) key for the gate read and the upsert); the RI-exchange config writer (handler_ri_exchange.go:2135) mutates only ri_exchange_* on the locked row, so the switch and stamps are preserved. No other SaveGlobalConfig/UpsertLadderConfig caller, no direct SQL writer, no seeding in migrations/terraform/cmd. PATCH /api/ladder/tranches/ does not touch arming fields.

Semantics. Grant = requireAutoPurchaseGrant (admin API key refused, execute:purchases checked by the real auth code, carved out of admin:* in auth/types.go:136). Modes in the pinned go pkg are exactly email_approval and auto_approve (ParseLadderMode exact match), so armed = enabled && mode=auto_approve is complete. Plain laddering_enabled, switch off, disarm, email_approval and lowering the cap stay ungated (tested with the admin key). Cap column is NUMERIC(20,6) (checked on PG17); LadderCapMicros rejects >6 decimals, <1e-6, >1e12, NaN/Inf; raise compared in exact micros; clearing the cap counts as a raise. The planner reads the cap back as float64 from the same <=6-decimal value, so no drift. Stamps json:"-" (forging test present), carried through read and upsert.

Local evidence (git archive at the exact SHA, GOTOOLCHAIN=go1.26.9, macOS, under the suite lock).

  • go build ./..., go vet, go test -count=1 ./...: exit 0.
  • Real PG17 (throwaway cluster, not mock): TestPostgresStoreDB_LadderArming PASS (fresh migrate: both switches OFF, NULL stamps; stamps survive a whole-config RI write; stored micros == compared micros for 8 caps). This test is SKIPPED in CI (env-gated); CI does apply 000116 on real Postgres (TestMigrations_FullStackIdempotent, TestPostgresStoreDB_GlobalConfig read and write the new columns, PASS).
  • Migration 000116 by hand on PG17: down, up, up again (idempotent, ADD COLUMN IF NOT EXISTS), columns nullable TEXT/TIMESTAMPTZ/TEXT; expand-only. 000116 still free on origin/main (c997ce6); no other open PR uses it.
  • git-secrets (--register-aws) BSD and GNU grep over all 15 changed files: exit 0.
  • Mutants (19, one run): 17 killed by test failures (switch gate removed, ladder_config gate removed, > to >=, clear-not-a-raise, admin key allowed on switch and on config, stamp dropped, armed ignores mode, armed ignores enabled, only-cap-gated, lookup error fails open, gate on on->off, auto-purchase gate dropped from the chain, before=after, 7 decimals allowed, stamp not saved, snapshot taken after unmarshal). 2 survived, both equivalent: unrepresentable-cap fallback (unreachable after Validate) and min bound >0 (every positive value below 1e-6 already has >6 decimals).

CI: 26/26 green at this SHA, mergeStateStatus CLEAN.

Non-blocking follow-ups.

  1. The ladder_config gate is read-then-upsert (as in feat(api): gate auto-purchase arming behind execute:purchases #841). Ruling: a concurrent write can only let a non-granted write restore arming values a granted user already authorized (it can never exceed the stored cap it read or arm a row that was never armed), for example re-arming a config that a granted user disarms in the same window. Fix later by reading the row FOR UPDATE in the upsert tx.
  2. No migration-level test for 000116 like 000113-000115 (coverage gap only; behaviour verified by hand above and through the CI full-stack migrate).
  3. execute:purchases constraints (account/provider) are not matched against the ladder config's account (same as feat(api): gate auto-purchase arming behind execute:purchases #841 plan arming); the executor PR should check this at fire time or the arming gate should pass the account.

@cristim
cristim merged commit 72fa7e4 into main Oct 13, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/few Limited audience priority/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant