fix(security): authorize the invoked operation against the authenticated principal

[cb1kenobi]

Fixes operation authorization in chooseOperation: authorize the invoked operation against the authenticated principal, and stop a caller-supplied search_operation from standing in as the permission subject.

What this fixes

verifyPerms reads both halves of the permission question off the object it's handed — the principal from hdb_user and the tables from schema/database/table/records. chooseOperation used to hand it json.search_operation, a caller-supplied field, making both halves body-controlled. Four issues, each with regression cover in integrationTests/security/choose-operation-authz.test.ts:

1. Cross-privilege redirect — verifyPerms ran against json.search_operation ?? json for every operation, but the handler runs against top-level json and only dataLayer/export.ts consumes search_operation. A non-super user could send a privileged top-level operation with a benign search_operation and get authorized against the benign tables. Now search_operation is the permission subject only for export_local/export_to_s3; every other op is checked against top-level json.
2. Principal smuggling — the nested principal was backfilled (if (!hdb_user)), honoring a body-supplied one. Now hdb_user is overwritten from the authenticated top-level principal unconditionally.
3. parsed_sql_object smuggling — the export worker re-reads parsed_sql_object (carrying permissions_checked) off the nested object, so a body-supplied one would execute an AST no check ever saw. It's now deleted at dispatch, forcing a re-parse + re-check in the worker.
4. Silent SQL denial — processAST's guard tested permissionsCheck.length > 0, but a denial is a PermissionResponseObject with no .length, so undefined > 0 was always false and denials executed. Fixed to if (permissionsCheck).

Non-object search_operation on an export op is now a 400 (was a wrapped 500), and the SQL AST check runs additively after verifyPerms rather than as an exclusive branch.

For the reviewer

• server/serverHelpers/serverUtilities.ts (chooseOperation) is the core. The load-bearing invariant: the object passed to verifyPerms must be the one the handler actually operates on. DESIGN.md (new chooseOperation section) documents the three rules and why each is required.
• export_local table check is currently unreachable — both export ops are requires_su and every path returns before the nested table check (super_user early-returns; a role granted the op via operations returns at gate 2; anything else is refused earlier). So the nested-table substitution is inert today. This is intentional for this patch.

Behavior change (release note)

Non-super users can no longer invoke export_local / export_to_s3 via a SQL search_operation. These operations are requires_su, and NoSQL export already enforced that ("Operation 'export_local' is restricted to 'super_user' roles"). SQL-based export previously took the SQL-only branch and skipped verifyPerms, so a non-super user could invoke the privileged export by wrapping it in a SQL search — governed only by table read perms. Routing export through verifyPerms closes that bypass and brings SQL export to parity with NoSQL export. Any non-super role that relied on SQL-export-without-export_local will now receive a 403. Three northwind export tests that encoded the old bypass were updated to assert the requires_su denial (they now mirror the existing NoSQL export case; they could be consolidated).

Deferred gaps (tracked, not fixed here)

Two pre-existing authorization gaps are left open deliberately — closing either changes authorization outcomes for existing role configurations, so each needs its own change + release note. Both are documented in DESIGN.md and pinned by regression tests so there's a test to flip:

• export_local/export_to_s3 via operations allowlist bypasses table-level grants (non-SQL nested search) #2215 — a role granted export_local/export_to_s3 via operations can export tables it holds no grant on (non-SQL nested search reaches the handler with no table check).
• Direct sql operation bypasses the operations allowlist (gate 1) #2216 — a direct sql operation bypasses the operations allowlist (gate 1); authorized only by the AST table check.

Verification

• integrationTests/security/choose-operation-authz.test.ts — pins the denial shapes and the current boundaries (principal override, nested-shape 400, forged-AST rejection, and the two deferred-gap boundaries NESTED-NOSQL / gate-1).
• Deep-review, multipass (2 independent runs × auth + SQL/AST lenses). Confirmed the fix closes the cross-privilege redirect, principal smuggling, AST smuggling, and the processAST denial-drop, with no other .length-style denial-drop anywhere in the tree. The multipass independently rediscovered the two deferred gaps above (now ticketed). One low cleanup nit surfaced: serverUtilities.ts:298 sets a top-level parsed_sql_object that's inert for the export path.

Review coverage

• Deep-review, multipass (Claude — Harper-domain + SQL/AST lenses, 2 independent runs): confirmed the fix closes the cross-privilege redirect, principal smuggling, AST smuggling, and the processAST denial-drop; no other .length-style denial-drop in the tree.
• Cross-model, Codex (graded, opposite-family): verdict COMMENTS, no production blockers. Two findings, both in the test file (below).
• Degraded legs (noted for honesty): Gemini agy failed (stochastic print-mode hang; CLI retries + 2 manual attempts all timed out) — no perf/maintainability lens this run; cursor-composer not authenticated locally; the CLI's Claude domain-adjudicator leg couldn't run (claude not on PATH) — covered instead by the separate multipass deep-review above.

Unresolved review findings (test-only, non-blocking)

• Codex, minor — integrationTests/security/choose-operation-authz.test.ts: no end-to-end case proves the export worker's nested-SQL recheck can deny an unauthorized query. The allowed super-user path reaching COMPLETE is covered, and unitTests/sqlTranslator/processAST.test.js proves the object-shaped denial stops routing, but the full dispatch → principal-propagation → reparse → denial path isn't exercised together. Worth adding a non-super-user unauthorized-export-SQL case that asserts denial.
• Codex, nit — same file: trim added comments that narrate tests / restate identifiers / address the reviewer (the suite catalog + run-command block at the top, and identifier restatements).

[gemini-code-assist]

Code Review

This pull request strengthens the security and authorization model of the operations API, specifically within chooseOperation. It ensures that the authenticated principal is always enforced and prevents body-supplied parameters (like nested hdb_user or parsed_sql_object) from bypassing authorization checks. Additionally, it fixes a critical bug in processAST where permission denials (which are objects lacking a length property) were not correctly blocked. The review feedback suggests a minor optimization in chooseOperation to avoid polluting the top-level request object with parsed_sql_object when executing nested SQL searches, as it is only consumed by direct SQL operations.

[gemini-code-assist]

The top-level json.parsed_sql_object is only consumed by direct SQL operations (where isSqlOperation is true) to avoid re-parsing in evaluateSQL. For nested SQL searches (like in export jobs), the worker executes evaluateSQL on the nested search_operation object (where parsed_sql_object is deliberately deleted to force a re-parse and re-check). Therefore, setting json.parsed_sql_object on the top-level json when isSqlOperation is false is inert and unnecessarily pollutes the request body. We should only assign it when isSqlOperation is true.

		if (isSqlOperation || hasNestedSqlSearch) {
			const sql = require('../../sqlTranslator/index');
			const sqlStatement = isSqlOperation ? json.sql : nestedSearch.sql;
			const parsedSqlObject = sql.convertSQLToAST(sqlStatement);
			if (isSqlOperation) {
				json.parsed_sql_object = parsedSqlObject;
			}

[claude]

Reviewed; no blockers found.