chore(api): low correctness findings from the 2026-09-02 audit #312

Summary

Four low-severity correctness defects in internal/api handlers, each confirmed against the audited commit: the single-instance cancel endpoint disables the whole plan against its documented contract, registration approval commits the status transition before the account insert, not-found on users, groups and API keys surfaces as 500, and enumerated query parameters are accepted unvalidated on the dashboard, inventory and accounts read paths.

Each item is independently actionable; they are grouped so one sitting can clear them.

Findings

• DELETE /api/purchases/planned/{id} disables the whole plan while the contract says the template stays intact (A01-021)

  • Location: internal/api/handler_purchases.go:429-433 (deletePlannedPurchase calling disablePlan at :492-517); contract at internal/api/types.go:697-706 and frontend/src/dashboard.ts:648-652 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: A user clicks Cancel on one upcoming purchase in the Home widget. The handler cancels the row and then sets plan.Enabled=false, so every future purchase of that plan stops until someone notices the toggle, while the type doc and the dashboard copy both say the plan template stays intact and the scheduler re-creates the next instance. The money-affecting behaviour (silently halting the ramp) is the undocumented one.
  • Evidence:

    	if canceled.PlanID != "" {
    		if err := h.disablePlan(ctx, canceled.PlanID); err != nil {
    			return nil, err
    		}
    	}

  • Suggested fix: Pick one contract: drop the plan-disable side effect from the single-instance cancel and let the Plans toggle own disabling, or update the UpcomingPurchase doc and the dashboard copy to say the plan is disabled.

• approveRegistration commits the approved transition before the account insert, so a failed insert strands the registration (A02-007)

  • Location: internal/api/handler_registrations.go:266-289; retry refused at :194-196 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: An admin approves a registration whose edited body carries an external_id that already exists. TransitionRegistrationStatus flips the row to approved, then CreateCloudAccount returns the duplicate-key error mapped to 409. The registration now reads approved with cloud_account_id = NULL, getPendingRegistration answers 409 "registration is already approved" on any retry, deleteRegistration is the only way out, and the registrant is never notified.
  • Evidence:

    reg.Status = "approved"
    h.setReviewMetadata(ctx, reg, httpReq)
    if err := h.config.TransitionRegistrationStatus(ctx, reg, "pending", validUUIDPtrOrNil(reg.ReviewedBy)); err != nil {
        ...
    }
    account := cloudAccountFromRequest(acctReq)
    ...
    if err := h.config.CreateCloudAccount(ctx, account); err != nil {
        return nil, classifyStoreError(err, ...)
    }

  • Suggested fix: Run the status transition and the account insert in one transaction, or on insert failure compare-and-set the row back from approved to pending before returning the error.

• Not-found and not-owner on users, groups and API keys surface as 500 (A02-012)

  • Location: internal/api/handler_users.go:108-110 (getUser), mapAuthError at :81-95; internal/api/handler_groups.go:90-93; internal/api/handler_apikeys.go:85-87,105-107; store returns pgx.ErrNoRows unchanged at internal/auth/store_postgres.go:772-775 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: GET /api/users/<random uuid> returns the raw pgx.ErrNoRows through Service.GetUser and the adapter; mapAuthError has no not-found case and IsNotFoundError matches only the package's own notFoundError type, so handleRequestError logs "API error" and answers 500 "Internal server error". The same holds for GET /api/groups/{id} (plain "group not found" error) and for deleting or revoking an API key that does not exist or belongs to another user, which the OpenAPI spec documents as 404. Each probe emits an error-level log line.
  • Evidence:

    user, err := h.auth.GetUser(ctx, userID)
    if err != nil {
        return nil, err
    }

  • Suggested fix: Export not-found / not-owner sentinels from internal/auth and map them to 404 in mapAuthError and the API key handlers, matching what openapi.yaml already promises.

• Enumerated query parameters are accepted unvalidated on several read paths (A02-021)

  • Location: internal/api/handler_dashboard.go:45-47,65 (getDashboardSummary); internal/api/handler_inventory.go:130-133 (fetchCommitmentRecords); internal/api/handler_accounts.go:210-219 (buildAccountFilter) at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: validateProvider is called only from the analytics, config, history and recommendations handlers. The dashboard forwards params["provider"] straight into ListRecommendations and fetchCommitmentPurchases, inventory compares it verbatim, and buildAccountFilter ignores any enabled value other than the literals true/false. provider=AWS or enabled=1 returns an empty or unfiltered result with 200 where the sibling endpoints return 400 for the same parameter.
  • Evidence:

    recommendations, err := h.scheduler.ListRecommendations(ctx, config.RecommendationFilter{
        Provider: params["provider"],
    })

  • Suggested fix: Call validateProvider in the dashboard and inventory handlers and reject unknown enabled values in buildAccountFilter.

Found by the 2026-09-02 codebase audit, findings A01-021, A02-007, A02-012, A02-021, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.