fix: bind inventory scope and constrain metric result keys

[Atom-oh]

Harden the inventory-summary and fleet-metric boundaries found while directly reviewing the pending main promotion (#137).

Inventory aggregate, split and EC2-distribution queries now reuse bound account/region arrays and the global-region flag instead of interpolating validated user values. Existing defaults, invalid-account fallback, explicit empty-region behavior and aggregate collection-status visibility are preserved.

Fleet metric results now use Maps internally, accept only requested metric keys and finite values, and serialize prototype-shaped entity names as ordinary own properties. Topology tests identify the exact origin ID instead of using host-substring predicates, removing misleading URL-sanitization patterns without changing application routing.

Validation: 154 targeted Vitest tests passed; 325 existing runtime-policy/session/audit/private-plan tests passed; npm ci and Next.js production build passed. Added coverage checks shared bindings across all three aggregate queries, prototype-shaped entity IDs, unexpected metric keys and non-finite responses. No infrastructure or feature flags changed.

[github-actions]

🤖 AI Code Review (Claude Fable 5 chair · lens×model matrix)

_Cells (model/lens): codex/L2 claude/L2 codex/L3 claude/L3 codex/L4 claude/L4 codex/L5 claude/L5 _

Status: PASSED — No blocking issues found

All load-bearing panel claims verified against base. The final chair review follows.

---

Chair Review — PR #139: fix: bind inventory scope and constrain metric result keys

IMAGE_COVERAGE: NOT_REQUIRED

1. Summary

This PR replaces the string-inlined account/region scope fragments in /api/inventory/summary with a single bound-parameter SCOPE_SQL ($1/$2/$3 reused across all UNION-ALL arms and both aggregates), and hardens fleetLatest in web/lib/metrics.ts against prototype pollution (Map-based accumulation), unrequested metric keys (allowlist via metrics.find), and non-finite values (Number.isFinite). Tests are tightened accordingly, including exact-id assertions in flow-topology.test.ts. I independently verified the semantic-equivalence truth table of SCOPE_SQL against the base accountCond/regionCond (all branches match, including the fail-closed empty-region case), confirmed PUBLIC_S3_WHERE contains no $n placeholders (no parameter collision), confirmed the base out[id][mm[1]] = v pollution vector existed and is reachable via user-supplied ?ids=__proto__, and confirmed the origin-node id format at flow-topology.ts:444. No CRITICAL or MAJOR findings survive; the change is a strict security and correctness improvement.

2. Issues per lens

Panel note: all four codex cells returned empty reviews (no findings, no analysis shown), while all four claude cells did detailed base-verified work. The codex "no findings" outcomes are consistent with the claude verdicts at the gate level (no CRITICAL/MAJOR anywhere), but they provide no independent corroboration of the minors below — those rest on claude-panel analysis, which I re-verified against base where load-bearing.

L2 — Code correctness

CRITICAL / MAJOR: None (2/2 models agree; claude-L2 enumerated every branch of the SCOPE_SQL truth table and the Map/Object.fromEntries semantics; I re-verified the key branches — __all__→NULL, all-invalid accounts→['self'], empty region selection→ANY('{}')→false, 'global' strip-then-fold ordering).

MINOR (claude-L2 only; codex-L2 silent):

• web/lib/metrics.ts:736-739 — a future …_i<digits>-shaped metric key now silently drops its datapoint (stays null) rather than writing a bogus key. Strictly better than base, but readEksMetricFleet's expected Id-map pattern (metrics.ts:1324) would make this loud and drop the per-result linear scan.
• web/app/api/inventory/summary/route.ts — the region charset filter in regionValues no longer serves an injection purpose and the summary route still duplicates scope logic instead of reusing accountWhereClause/regionWhereClause from web/lib/inventory.ts (see also L4 Sync the public samples tree to Atom-oh/awsops@68b3f58f #2).
• SCOPE_SQL's $N IS NULL branches fail open by construction; today null is only produced by the deliberate __all__ branch — worth one comment line pinning that invariant.
• splitsSql() is now a nullary function returning a constant; could be a module-level const.

L3 — Security / AWS mutation safety

CRITICAL / MAJOR: None (2/2 models agree). ADR-005 boundary intact: the only AWS call touched is read-only GetMetricData; no IAM/Terraform/secret changes. Both security-relevant changes are strict tightenings, verified: SQL scope binding preserves every fail-closed branch of the removed fragments, and the prototype-pollution fix closes a genuinely request-reachable vector (/api/inventory/[type]/metrics ?ids= validation admits __proto__; base out[id] wrote through Object.prototype) for all five fleetLatest-delegating fleet functions.

MINOR (claude-L3, independently re-derived by claude-L4 under its lens; codex silent — I confirmed against base metrics.ts:601-632):

• ec2FleetLive retains the exact pre-fix pattern: out[id] ??= {...} skips seeding when id === '__proto__' (Object.prototype is truthy), and the subsequent unallowlisted (out[id] as Record<string, number>)[mm[1]] = … write lands on Object.prototype. Not request-reachable today (its only caller feeds inventory-sourced instance ids), but it is the same defect class this PR fixes in the same file — align it now.
• The retained 'self' | 12-digit / region-charset filters are now defense-in-depth, but the comments explaining why they exist were deleted; a future cleanup could remove them as "redundant since parameterized."

L4 — Observability / data-integration correctness

CRITICAL / MAJOR: None (2/2 models agree; claude-L4's branch-by-branch equivalence table matches my own verification, including schema.sql's region TEXT NOT NULL DEFAULT '' ruling out NULL-region semantics drift).

MINOR (claude-L4; codex silent):

1. ec2FleetLive inconsistency (shared with L3 above), plus: it still surfaces non-finite values as NaN/Infinity on EC2 tiles while every other fleet path now reports null — "unknown" is defined inconsistently across fleets.
2. Account-fallback drift vs. the rows route: ?accounts=invalid → ['self'] here (host-account counts) but accountWhereClause passes ['invalid'] through → 0 rows — the same counts-vs-rows drift class gap L110 exists to prevent, and the new it.each case now pins it as intended. Pre-existing behavior, so excluded from the gate, but reusing the shared helpers would make parity structural.
3. The repeated ($1 IS NULL OR …) guard across 17 UNION arms changes the planner's input vs. literal IN-lists; worth one EXPLAIN (ANALYZE) on a full-fleet dataset before merge (node-pg unnamed statements get value-aware plans, so likely fine).
4. The flow-topology test changes (correct and stricter — id format verified at flow-topology.ts:444) incidentally drop the only label-content assertions in three cases; add one expect(o.label) alongside an id lookup.

L5 — Docs/ADR consistency

CRITICAL / MAJOR: None (2/2 models agree). No doc asserts the old "validated then inlined" mechanism; docs/api-reference.md's scope description remains accurate; the empty [Unreleased] CHANGELOG is consistent with the repo convention since there is no net user-visible behavior change (per root CLAUDE.md's rule, not flagging this).

MINOR (claude-L5; codex silent):

• The deleted regionCond comment was the only in-code statement of the gap-L110 counts-must-match-rows-route contract, still cited by web/app/inventory/[type]/page.tsx:103 and docs/v1-gap-audit-2026-07-19.md:323; restore one line so the audit trail stays traceable.
• The fleetLatest contract comment (metrics.ts:697-698, "every failure degrades to nulls") doesn't mention the two new rules (unrequested-Id ignore, non-finite→null), which matters because the file explicitly documents the legacy-vs-strict split against readEksMetricFleet.

3. Suggestions

1. Apply the same Map/allowlist/Number.isFinite hardening to ec2FleetLive (metrics.ts:601-632) — same file, same defect class, cheap now (L3+L4 convergent).
2. Add one-line comments: (a) the retained account/region validation is defense-in-depth on top of binding; (b) null scope values come only from __all__; (c) restore the gap-L110 parity reference; (d) note the two new fleetLatest filtering rules.
3. Consider replacing accountValues/regionValues with the shared accountWhereClause/regionWhereClause helpers (or reconciling the invalid-account fallback) so counts/rows parity is structural.
4. Hoist splitsSql to a module constant; add a label assertion in one flow-topology origin test; run one EXPLAIN (ANALYZE) on the summary aggregation against a realistic dataset.

4. Verdict

No CRITICAL or MAJOR findings from any of the 8 panel cells, and none arose from chair verification. All minors are hardening/documentation follow-ups, several pre-existing and outside the diff's gate scope. Image manifest is empty (no PNG evidence required); all cells plus chair declared coverage NOT_REQUIRED.

---

Triggered by commit f566ac556fc45612e6d64fa6f155de7c644a37f2 · workflow: .github/workflows/pr-review.yml