Adversarial review of all high-stakes merged PRs — findings tracker
An independent adversarial review swept all 8 high-stakes subsystems (407 of 581 merged PRs) against current committed main, reviewing each subsystem holistically (not per-diff). Every finding below was confirmed to persist in current main at review time (old-PR flaws already fixed by later PRs were excluded). This issue tracks the full set; each fix lands as its own follow-up PR with a regression test.
🔴 Critical / High — fix-forward PRs in progress
🟡 Medium — fix-forward in progress
🟠 Medium/Low — to be scheduled (not yet in a fix PR)
🔎 Needs LIVE cloud verification (cannot confirm without real credentials)
✅ Notable clean subareas (traced, no defects)
pkg/ladder (allocation/tranche/guardrails); auth/OIDC (RS256-pinned inbound + subject pinning, constant-time everywhere, HKDF CSRF, fail-closed MFA/login, race-free last-admin trigger, MaskToken); approval tokens + atomic claim + submit idempotency + ramp-step + creator-scope + delayed-fire; no double-amortization; snapshot SUM-then-AVG rollups; active-only SQL reads; account-scope on listing endpoints; AWS pagination/tenancy/RDS-enum; Azure httpclient.New() on all clients + fabricated-price refusal; IaC IAM bootstrap-vs-runtime split + PassRole/ExternalId/OIDC scoping (except the Fargate SES gap).
Findings from the 2026-09-02 codebase audit
Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.
A09-004 (critical)
H1-H4 has a third site the bullet does not name: pkg/exchange/exchange.go:303-308. resolvePaymentDue substitutes a fresh zero big.Rat whenever PaymentDueUSD is nil, which happens whenever the AWS response's PaymentDue is empty (the parse at :260 is gated on a non-empty raw string). checkInitialQuote (:313) and checkReQuote (:330) then compare that zero against MaxPaymentDueUSD and always pass, so an exchange AWS declined to price reaches AcceptReservedInstancesExchangeQuote (:372) with no effective ceiling. The doc comment asserts the only cause of a nil is a genuinely zero-cost exchange; nothing verifies that. Finding A09-004.
A05-003 (medium)
Detail and a narrowing for the Purchase F1 line, from finding A05-003. The mechanism is that enforceFourEyesPolicy (internal/purchase/approvals.go:167) documents itself as the universal choke point but runs only inside ApproveAndExecute. claimAndExecute (manager.go:177-193) calls executeAndFinalize directly, so both of its callers, the SQS execute_purchase worker (messages.go:136) and the cron sweep processOneExecution (manager.go:678), commit money without reaching the gate. The four-eyes tests cover ApproveAndExecute and ProcessMessage's approve branch only. Narrowing worth recording: the human self-approval scenario is not reachable today, because executableByScheduler rejects PurchaseSourceWeb (manager.go:635) and the only two writers of CreatedByUserID both stamp Source=cudly-web. What is reachable is a system-created row (nil creator, empty Source, AutoPurchase plan) executing on the cron tick where checkDifferentApprover would have denied it fail-closed. So the fix is to decide whether dual control binds AutoPurchase and say so at the gate, moving the check into executeAndFinalize alongside armedRedriveRefusal if it does.
A07-022 (medium)
Exchange M2 has a sibling on the ListTargetOfferings path. normalizeTargetOfferingsParams (providers/aws/services/ec2/client.go:874-878) calls convertEC2PaymentOption and discards the error with if ... err == nil, so an unrecognised payment option (for example the display form "All Upfront" rather than "all-upfront") leaves offeringType at its zero value. The SDK omits an empty offeringType, so AWS returns offerings for every payment option and ListTargetOfferings presents partial-upfront and all-upfront exchange targets for a request that asked for one specific option, with no signal to the caller that its input was rejected. The path is live from internal/api/handler_ri_exchange.go:154. Audit finding A07-022.
A07-023 (medium)
Exchange M2 undersells this one: alongside the payment option and the first-result nondeterminism, FindConvertibleOffering (providers/aws/services/ec2/client.go:794-818) does not paginate at all. It issues a single DescribeReservedInstancesOfferings with a six-entry Filters[] request, MaxResults: 20, no OfferingType, and returns ReservedInstancesOfferings[0]. The comment on findOfferingID at :481-487 documents that this exact Filters[] shape is the one AWS answers with empty pages plus a NextToken on sparse offering sets, so a first empty page yields 'no convertible offering found' for an instance type that does have one. Live callers are internal/server/ladder_write.go:64 and internal/server/handler_ri_exchange.go:63. Whatever fixes the payment-option match should also build the request from typed fields the way describeInputFromQuery does and page with the existing isLastEC2Page helper. Audit finding A07-023.
A09-010 (medium)
Exchange M4 has a neighbour in the same loop. sizeOrder ends with "metal" (pkg/exchange/reshape.go:382-387) and normalizationFactors["metal"] == normalizationFactors["24xlarge"] == 192 (:378-379), and the loop at :694-699 keeps the LAST index whose factor fits, so any normalizedUsed >= 192 selects index 17, "metal". analyzeRI then composes family + "." + targetSize at :486 with no per-family existence check, producing targets such as m5.metal, t3.9xlarge and r5.3xlarge. resolveOffering in auto.go fails the lookup and the recommendation is silently skipped, while the reshape dashboard shows a target no operator can act on. Dropping "metal" from sizeOrder (keeping it in normalizationFactors for parsing existing RIs) and preferring the smaller-indexed size on a factor tie fixes both. Audit finding A09-010.
A09-011 (medium)
A second route into the M4 outcome, upstream of findBestFit. normalizationFactors["metal"] = 192 (pkg/exchange/reshape.go:379) is one family-independent entry for a quantity that is family-dependent: m5.metal is 24xlarge-equivalent (192), but i3.metal is 16xlarge-equivalent (128) and m5zn.metal is 12xlarge-equivalent (96). resolveNormFactor (:432-437) falls back to that table whenever ri.NormalizationFactor == 0, feeding normalizedPurchased at :461 and findBestFit at :481 with no family discrimination, so an i3.metal at 50% utilization computes normalizedUsed 96 instead of 64 and the target is sized 50% too large: more capacity than owned, which is M4's stated failure. Per the fail-loud rule the entry should come out so resolveNormFactor returns 0 and analyzeRI skips the RI unless the caller supplied the real per-family factor from ec2:DescribeInstanceTypes. Audit finding A09-011.
A04-018 (low)
The Migration#1 entry in this tracker ("dirty auto-heal marks a rolled-back migration as applied") is premised on a Force that no longer happens, so it needs re-deriving before anyone works it. maybeAutoHealDirty (internal/database/postgres/migrations/migrate.go:381-421) calls no Force at all; it returns an error on every dirty row. The names and comments were not updated with it: the gate is still CUDLY_MIGRATION_AUTOHEAL, autoHealEnabled still defaults it to true (:428), and the call-site comment at :114-118 still promises the dirty flag is cleared "at the CURRENT recorded version so the subsequent Up() re-applies any pending migrations, letting a cold start self-recover". So the live risk has inverted: rather than a wrong auto-heal, an operator who leaves the gate at its true default expects self-recovery and gets a permanently dirty database until they discover CUDLY_FORCE_MIGRATION_VERSION. Renaming to maybeRefuseDirty / CUDLY_MIGRATION_DIRTY_CHECK and rewriting the comment would make the tracker item resolvable as no-longer-applicable. Audit finding A04-018.
Adversarial review of all high-stakes merged PRs — findings tracker
An independent adversarial review swept all 8 high-stakes subsystems (407 of 581 merged PRs) against current committed
main, reviewing each subsystem holistically (not per-diff). Every finding below was confirmed to persist in current main at review time (old-PR flaws already fixed by later PRs were excluded). This issue tracks the full set; each fix lands as its own follow-up PR with a regression test.🔴 Critical / High — fix-forward PRs in progress
SavingsPlansDetails;lookupOfferingIDreturns the lexicographically-smallest offering across families; uses client region not the rec's. (parser_sp.go,savingsplans/client.go)purchase_history.termis years buthandler_marketplace.gotreats it as months; supplied schedules rejected. Frontend consent modal (history.ts:1210) mirrors it.handler_ri_exchange.go,pkg/exchange/auto.go)revoked_at IS NULLon read paths).by_service.current_savingsfabricated as full potential for services with no commitments (competing Merge per-service savings charts into one range chart with current-savings underlay; fix unpopulated current_savings #908 fixes; tests cement it).pending/notifiedwith no approval + noAutoPurchasegate.resourceType); Synapse filters non-existent'SQLDatabaseDTU'(+ database'SqlDatabase', cosmosdb'CosmosDb'casing) — being fixed against SDK enums.🟡 Medium — fix-forward in progress
'cancelled';canceled_bywritten by no code; RI-exchange writers legacy — completes the expand-contract before fix(db): contract migration -- drop 'cancelled' from CHECK constraints and drop cancelled_by column cloud-commitments-platform#47./api/historyexempts empty-AccountIDrows → leaks other users' in-flight purchases + emails (PII).ses:FromAddresscondition the Lambda role has).TestMigrations_FullStackIdempotentis a false guard (2nd run is a version-table no-op).ExpandPaymentVariants(fabricates&0.0); AWS SP parser coerces unparseable money to 0 (RI path fails loud).🟠 Medium/Low — to be scheduled (not yet in a fix PR)
ec2/client.go FindConvertibleOffering)ec2/client.go getDurationValue)findBestFitcan recommend MORE capacity than owned (reshape increases spend)GetOfferingDetailsreads upfront from marketplacePricingDetails→ $0 for standard offerings000032...down.sqlfails on realistic term/payment-variant data (chained-rollback dead end)scheduled_execution_atEngine:"redis",SavingsPlanDetails.Coveragemislabel, Azure retail-pricing silent truncate, region-name aliases, memorydb pagination no ctx cap), auth low (JWKShttp.DefaultClientnot hardened; recovery-code modulo bias), RBAC F2 (API-key constraints not enforced — being fixed).🔎 Needs LIVE cloud verification (cannot confirm without real credentials)
SUBSCRIPTION_IDgroup key but matched against EC2ReservedInstancesId— if mismatched, the entire reshape/auto-exchange pipeline silently no-ops ("all RIs well-utilized"). Noutilization_test.go/ fixture. Verify one realGetReservationUtilizationkey shape; extract RI id fromreservationARNif needed.google.billing.CostInsight.commitmentRecommender(an insight type, not a recommender; commit1778a2f7bchanged AWAY from the documentedgoogle.compute.commitment.UsageCommitmentRecommender); cloudsql/memorystore use Performance recommenders (recommend cost increases, mislabeled as CUD savings). If invalid, GCP CUD recs silently show "none". Verify each viagcloud recommender recommendations list --recommender=....SqlDataWarehouse/SQLDatabases/CosmosDBfilter values against a live subscription (SDK-enum check done in the fix PR; live behavior still worth confirming).✅ Notable clean subareas (traced, no defects)
pkg/ladder(allocation/tranche/guardrails); auth/OIDC (RS256-pinned inbound + subject pinning, constant-time everywhere, HKDF CSRF, fail-closed MFA/login, race-free last-admin trigger,MaskToken); approval tokens + atomic claim + submit idempotency + ramp-step + creator-scope + delayed-fire; no double-amortization; snapshot SUM-then-AVG rollups; active-only SQL reads; account-scope on listing endpoints; AWS pagination/tenancy/RDS-enum; Azurehttpclient.New()on all clients + fabricated-price refusal; IaC IAM bootstrap-vs-runtime split + PassRole/ExternalId/OIDC scoping (except the Fargate SES gap).Findings from the 2026-09-02 codebase audit
Added by an automated audit of
3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd(tip oforigin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report:docs/audits/codebase-audit-2026-09-02.md.A09-004 (critical)
H1-H4 has a third site the bullet does not name: pkg/exchange/exchange.go:303-308. resolvePaymentDue substitutes a fresh zero big.Rat whenever PaymentDueUSD is nil, which happens whenever the AWS response's PaymentDue is empty (the parse at :260 is gated on a non-empty raw string). checkInitialQuote (:313) and checkReQuote (:330) then compare that zero against MaxPaymentDueUSD and always pass, so an exchange AWS declined to price reaches AcceptReservedInstancesExchangeQuote (:372) with no effective ceiling. The doc comment asserts the only cause of a nil is a genuinely zero-cost exchange; nothing verifies that. Finding A09-004.
A05-003 (medium)
Detail and a narrowing for the Purchase F1 line, from finding A05-003. The mechanism is that
enforceFourEyesPolicy(internal/purchase/approvals.go:167) documents itself as the universal choke point but runs only insideApproveAndExecute.claimAndExecute(manager.go:177-193) callsexecuteAndFinalizedirectly, so both of its callers, the SQSexecute_purchaseworker (messages.go:136) and the cron sweepprocessOneExecution(manager.go:678), commit money without reaching the gate. The four-eyes tests coverApproveAndExecuteandProcessMessage's approve branch only. Narrowing worth recording: the human self-approval scenario is not reachable today, becauseexecutableBySchedulerrejectsPurchaseSourceWeb(manager.go:635) and the only two writers ofCreatedByUserIDboth stampSource=cudly-web. What is reachable is a system-created row (nil creator, empty Source, AutoPurchase plan) executing on the cron tick wherecheckDifferentApproverwould have denied it fail-closed. So the fix is to decide whether dual control binds AutoPurchase and say so at the gate, moving the check intoexecuteAndFinalizealongsidearmedRedriveRefusalif it does.A07-022 (medium)
Exchange M2 has a sibling on the ListTargetOfferings path. normalizeTargetOfferingsParams (providers/aws/services/ec2/client.go:874-878) calls convertEC2PaymentOption and discards the error with
if ... err == nil, so an unrecognised payment option (for example the display form "All Upfront" rather than "all-upfront") leaves offeringType at its zero value. The SDK omits an empty offeringType, so AWS returns offerings for every payment option and ListTargetOfferings presents partial-upfront and all-upfront exchange targets for a request that asked for one specific option, with no signal to the caller that its input was rejected. The path is live from internal/api/handler_ri_exchange.go:154. Audit finding A07-022.A07-023 (medium)
Exchange M2 undersells this one: alongside the payment option and the first-result nondeterminism,
FindConvertibleOffering(providers/aws/services/ec2/client.go:794-818) does not paginate at all. It issues a singleDescribeReservedInstancesOfferingswith a six-entryFilters[]request,MaxResults: 20, noOfferingType, and returnsReservedInstancesOfferings[0]. The comment onfindOfferingIDat :481-487 documents that this exactFilters[]shape is the one AWS answers with empty pages plus aNextTokenon sparse offering sets, so a first empty page yields 'no convertible offering found' for an instance type that does have one. Live callers are internal/server/ladder_write.go:64 and internal/server/handler_ri_exchange.go:63. Whatever fixes the payment-option match should also build the request from typed fields the waydescribeInputFromQuerydoes and page with the existingisLastEC2Pagehelper. Audit finding A07-023.A09-010 (medium)
Exchange M4 has a neighbour in the same loop.
sizeOrderends with"metal"(pkg/exchange/reshape.go:382-387) andnormalizationFactors["metal"] == normalizationFactors["24xlarge"] == 192(:378-379), and the loop at :694-699 keeps the LAST index whose factor fits, so anynormalizedUsed >= 192selects index 17,"metal".analyzeRIthen composesfamily + "." + targetSizeat :486 with no per-family existence check, producing targets such asm5.metal,t3.9xlargeandr5.3xlarge.resolveOfferingin auto.go fails the lookup and the recommendation is silently skipped, while the reshape dashboard shows a target no operator can act on. Dropping"metal"fromsizeOrder(keeping it innormalizationFactorsfor parsing existing RIs) and preferring the smaller-indexed size on a factor tie fixes both. Audit finding A09-010.A09-011 (medium)
A second route into the M4 outcome, upstream of
findBestFit.normalizationFactors["metal"] = 192(pkg/exchange/reshape.go:379) is one family-independent entry for a quantity that is family-dependent:m5.metalis 24xlarge-equivalent (192), buti3.metalis 16xlarge-equivalent (128) andm5zn.metalis 12xlarge-equivalent (96).resolveNormFactor(:432-437) falls back to that table wheneverri.NormalizationFactor == 0, feedingnormalizedPurchasedat :461 andfindBestFitat :481 with no family discrimination, so ani3.metalat 50% utilization computesnormalizedUsed96 instead of 64 and the target is sized 50% too large: more capacity than owned, which is M4's stated failure. Per the fail-loud rule the entry should come out soresolveNormFactorreturns 0 andanalyzeRIskips the RI unless the caller supplied the real per-family factor fromec2:DescribeInstanceTypes. Audit finding A09-011.A04-018 (low)
The Migration#1 entry in this tracker ("dirty auto-heal marks a rolled-back migration as applied") is premised on a Force that no longer happens, so it needs re-deriving before anyone works it.
maybeAutoHealDirty(internal/database/postgres/migrations/migrate.go:381-421) calls noForceat all; it returns an error on every dirty row. The names and comments were not updated with it: the gate is stillCUDLY_MIGRATION_AUTOHEAL,autoHealEnabledstill defaults it to true (:428), and the call-site comment at :114-118 still promises the dirty flag is cleared "at the CURRENT recorded version so the subsequent Up() re-applies any pending migrations, letting a cold start self-recover". So the live risk has inverted: rather than a wrong auto-heal, an operator who leaves the gate at its true default expects self-recovery and gets a permanently dirty database until they discoverCUDLY_FORCE_MIGRATION_VERSION. Renaming tomaybeRefuseDirty/CUDLY_MIGRATION_DIRTY_CHECKand rewriting the comment would make the tracker item resolvable as no-longer-applicable. Audit finding A04-018.