feat/one-api

Null sealing key permanently cached on SM transient failure at cold start

apps/platform/src/lib/secrets/sealing-key.ts

If Secrets Manager returns null (transient error) AND SEALING_KEY is absent, decodeKey('') → 0-byte buffer → null, cached = null. The process-wide cache means a Lambda that hits a transient SM error on its first invocation will permanently return null until it is killed. The old code shared this flaw, but this commit removes the env-var fast path that could have bypassed SM entirely. Consider not caching negative results so transient errors trigger a retry.

showAll is NOT a dead parameter — PATH 2 removal just changed its meaning

apps/platform/src/api/contracts/bills.contract.ts

showAll is still actively read in listBillsHandler, billAggregatesHandler, billFacetsHandler, and CSV export — it controls the isMonitored=true default filter. The contract comment update in this diff correctly reflects this. No dead parameter issue.

ContextFCIS removal is clean

apps/platform/src/api/handlers/bills.handler.ts

ContextFCIS is not referenced anywhere in bills.handler.ts or csv/route.ts. Remaining uses in contexts.handler.ts and utility-contracts.handler.ts are for separate capabilities. Clean removal.

Deep import @batu/core-domain/lib/sealed-secret is valid

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts

The sub-path ./lib/sealed-secret is in @batu/core-domain's package.json exports field. The dynamic import resolves correctly at Lambda runtime.

No CloudTrail alert on sealing-key Secrets Manager reads

apps/platform/src/lib/secrets/sealing-key.ts

The comment correctly notes that SM gates reads behind IAM and records them in CloudTrail. Worth confirming a CloudTrail-to-alerting pipeline exists for the sealing-key secret ARN specifically, since this key opens every customer's webhook signing secret.

SEALING_KEY_ID env var override in Lambda could silently break all webhook deliveries

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts

The Lambda still reads process.env['SEALING_KEY_ID'] ?? CURRENT_SEALING_KEY_ID. If an operator sets SEALING_KEY_ID to any wrong value, open() returns null and every delivery is silently skipped. Since the constant is now centralized, this override has no legitimate use case in deployed environments — consider removing it or documenting in the CDK stack that it must never be set.

SM failure is silently swallowed — no log for operators to distinguish config vs transient error

apps/platform/src/lib/secrets/sealing-key.ts

readFromSecretsManager() catches all exceptions and returns null, including IAM permission errors and throttling. There is no logging in the catch block. An operator cannot distinguish 'SM is not configured' from 'SM errored' without a CloudWatch metric or log line.

Resolution order change is strictly more secure — confirmed

apps/platform/src/lib/secrets/sealing-key.ts

Secrets Manager first is confirmed correct: SM gates reads behind IAM and CloudTrail. decodeKey('') safely returns null (0-byte buffer). Fail-closed on both absent is correct behavior for a key that protects every customer's webhook secret. The org isolation chain for service-role bills/contracts reads is sound: ID set built server-side from org → sites → SUC, never from query params.

Stale @param profileId in getAccessibleContracts JSDoc

apps/platform/src/api/handlers/bills.handler.ts

The JSDoc still reads `@param profileId - User's profile ID` but the function signature is `getAccessibleContracts(orgId: string)` — it never accepted a profileId. The doc is actively wrong on the parameter name.

CURRENT_KEY_ID alias adds indirection with no clarity

apps/platform/src/lib/secrets/sealing-key.ts

const CURRENT_KEY_ID = CURRENT_SEALING_KEY_ID is a local alias used one line below. The comment misdescribes it as a re-export. Using CURRENT_SEALING_KEY_ID directly is cleaner and self-documenting.

Eight near-identical SG-19 comment blocks — high repetition, single edit point risk

apps/platform/src/api/handlers/bills.handler.ts

The 5-line SG-19 CLOSED block appears ~8 times across four handler files. The WHY is non-obvious so a comment is warranted, but a one-liner at each site plus a single canonical explanation in domains/utility/CLAUDE.md § RESOLVED would reduce future maintenance burden.

accessible-contracts.ts docstring is accurate

packages/api/src/auth/accessible-contracts.ts

The updated @returns correctly describes: one path (sites → SUC); PATH 2 removed with production evidence; service-role is a migration step not a constraint. Accurate and appropriately detailed for a security-adjacent authorization file.

showAll contract comment accurately updated

apps/platform/src/api/contracts/bills.contract.ts

The comment was updated from 'include PATH 2 (entity_relationship) contracts and all isMonitored values' to 'drop the default isMonitored=true filter (full history view)'. This matches the actual remaining behavior. Accurate.

Platform sealing-key resolution order is completely untested

apps/platform/src/lib/secrets/sealing-key.ts

__resetSealingKeyCacheForTests is exported but called by no test file. None of the three critical scenarios are verified: (a) SM returns a key → env var is ignored; (b) SM returns null → env var IS used; (c) both absent → returns null (fail-closed). A future refactor could flip the priority order back without any test catching it.

showAll / isMonitored filter branching in bill handlers has no tests

apps/platform/src/api/handlers/bills.handler.ts

The showAll branching that gates whether unmonitored contracts appear in results is untested. No test exercises showAll=true or showAll=false on listBillsHandler, billAggregatesHandler, or billFacetsHandler. Behavioral regression here would go undetected.

No round-trip test asserting seal and open share the same CURRENT_SEALING_KEY_ID

domains/core/src/lib/sealed-secret.ts

The constant 'k1' is now the single source of truth, but no test exercises the full cross-boundary path: platform seal() using CURRENT_KEY_ID → Lambda loadSealingKey() resolving the same CURRENT_SEALING_KEY_ID. A mismatch ('k1' vs 'K1') would mean open() returns null and every delivery is silently skipped.

Lambda dynamic import of CURRENT_SEALING_KEY_ID not verified against package exports map

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts

await import('@batu/core-domain/lib/sealed-secret') resolves in Vitest (TypeScript source) but could fail at Lambda runtime if @batu/core-domain's package.json exports map does not expose lib/sealed-secret as an entry point. Worth a quick verify.

Stale inline comment at listBillsHandler still references removed PATH 2

apps/platform/src/api/handlers/bills.handler.ts

Line ~262 reads `// When showAll=true, also fetch PATH 2 (entity_relationship) contracts` immediately before the call to `getAccessibleContracts(orgResult.value.orgId)` which no longer touches entity_relationships. showAll still exists (for the isMonitored default), but the PATH 2 side-effect is gone. Should read: `// showAll controls the isMonitored default below — PATH 2 was removed`.

decodeKey(process.env.SEALING_KEY ?? '') implicitly relies on zero-byte failure

apps/platform/src/lib/secrets/sealing-key.ts

When SEALING_KEY is unset, '' is passed to decodeKey and relies on Buffer.from('', 'base64') being 0 bytes → failing the length check. Explicit guard: `process.env.SEALING_KEY ? decodeKey(process.env.SEALING_KEY) : null` is clearer and immune to any future relaxation of the length check.

Dynamic import in webhook-dispatcher is the established Lambda pattern here

packages/event-bus/src/handlers/webhook-dispatcher.lambda.ts

Consistent with existing await import('@batu/core-domain/webhook-endpoint') and await import('@batu/database') in the same file — this is the declared esbuild bundling pattern. Reason for dynamic vs static import is not documented; a one-line comment would help future readers.