fix(security): reject SELECT..INTO / volatile-function bypass of the SQL read-only gate + clear base Semgrep + JWKS scheme hardening

[seonghobae]

Base-branch security hardenings, verified against tests/test_api.py.

1. SQL read-only gate bypassed by SELECT..INTO / volatile functions — Medium (commit 5438fc7)

validate_sql_query enforced its documented "read-only SELECT only" guarantee with a prefix test (startswith("select ")) and never inspected the body, so two write/side-effect constructs that begin with the token SELECT slipped past the safety gate:

• SELECT * INTO exfil FROM crm — PostgreSQL SELECT INTO is CREATE TABLE AS, a write/DDL. Verified end-to-end through /browse/query: HTTP 200 SUCCEEDED (dataset crm-event, whose only allowlisted table is crm).
• SELECT pg_sleep(10) FROM crm (and pg_read_file / pg_write_file / pg_ls_dir / lo_import / lo_export / dblink) — volatile / file-access function side effects and DoS.

Execution is mocked today, but validate_sql_query is the designated pre-execution safety gate for the real query engine, and its SELECT-only guarantee is asserted to callers (draft_sql). Fix: an into write-reject plus an _UNSAFE_FUNCTIONS reject set, added after the forbidden-keyword scan and before the table-extraction block so it composes cleanly with the comma-join allowlist fix in #35 (a different vector — confirmed #35 does not touch the read-only check).

• Tests: end-to-end /browse/query returns 400 for SELECT..INTO (write_operation_not_allowed), plus a unit test covering INTO, pg_sleep, pg_read_file, and a clean-SELECT no-warning control. Verified red→green (without the fix the bypass tests fail). Full test_api.py: 73 passed.
• Dedup: not a duplicate. fix(orchestrator): close SQL source-table allowlist bypass via comma joins #35 reworks which tables are extracted (comma-join bypass); fix(ci): stop the nightly Atheris OOM + clear 3 reviewed Semgrep base findings #33/fix(sdp): unblock CI — fuzz OOM + crash-log visibility + base SAST remediation #34/fix(security): clear the scheduled fuzz OOM and the 3 Semgrep SAST findings at base #36 are fuzz/Semgrep base work. No open PR addresses the INTO/read-only vector.

2. Base Semgrep gate + JWKS scheme hardening (earlier commit)

Reviewed # nosemgrep annotations clearing the base Semgrep gate, plus a JWKS URL-scheme guard in authz.py.

Verification

PYTHONPATH=src pytest tests/test_api.py     # 73 passed

The governance invariant is preserved: every data path still calls policy.evaluate(...) before returning data and records a policy decision + audit event.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH

[coderabbitai]

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 113 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8e15f0d0-1ce7-4c9b-a5b0-3552446c0378

📥 Commits

Reviewing files that changed from the base of the PR and between e48aa13 and 76fcfb6.

📒 Files selected for processing (4)

• docs/doctoring/sql-readonly-function-admission.md
• src/sdp/orchestrator.py
• tests/test_api.py
• tests/test_sql_readonly_function_gate.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[opencode-agent]

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

• Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

• Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

• Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

• Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

• Result: REQUEST_CHANGES

• Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 736ba39674cb44ca41326430c4743e85baecc722.

• Head SHA: 736ba39674cb44ca41326430c4743e85baecc722

• Workflow run: 30464111091

• Workflow attempt: 1

Coverage evidence

Coverage Decision

• Result: FAIL
• Test evidence: not proven passing
• Docstring evidence: not proven passing when configured
• Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test_api.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_api.py"]
  R2 --> V2["targeted test run"]

Loading

[opencode-agent]

OpenCode Review Overview

• Head SHA: 76fcfb69664a2de49754cc3a94ebd69b5e273870
• Workflow run: 31833933039
• Workflow attempt: 1
• Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

• Problem: Failed same-head checks remain for 76fcfb69664a2de49754cc3a94ebd69b5e273870.
• Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
• Fix: Read and fix the failed check logs below, then rerun the current-head checks.
• Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

• Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31738831114/job/94576914593)
• trivy-fs check run: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31738831114/job/94576914593)

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Docs: sql-readonly-function-admission.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: sql-readonly-function-admission.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file: orchestrator.py"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: orchestrator.py"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (2 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (2 files)"]
  R3 --> V3["targeted test run"]

Loading

[seonghobae]

Blocker note (not fixable in this PR): every required mechanical check on this head is green — Semgrep now passes (the base findings this PR fixes: dynamic-urllib-use-detected ×2 + sqlalchemy-execute-raw-query) plus the rest. The only thing withholding approval is the central OpenCode coverage-evidence sandbox, which fails for reasons unrelated to this diff:

cd . && PYTHONPATH=. python3 -m coverage run -m pytest tests
ERROR tests/test_design_tokens.py  -> ModuleNotFoundError: No module named 'sdp'
ERROR tests/test_api.py            -> No module named 'jwt'
ERROR tests/test_graph_engine.py   -> No module named 'fastapi'
ERROR tests/fuzz/...               -> No module named 'pydantic'

Two central-pipeline defects, not this PR's content: (1) the sandbox materialized an empty dep set (pydantic/jwt/fastapi never installed), and (2) it runs with PYTHONPATH=. while this repo is src-layout (sdp lives under src/, so import sdp needs PYTHONPATH=src). The org ruleset then blocks merge on the resulting REQUEST_CHANGES. Tracked for the central materialize_base_python_requirements / sandbox-PYTHONPATH fix (same class as ContextualWisdomLab/.github#612); this PR will re-review clean once the sandbox installs deps and uses the correct import path.

---

Generated by Claude Code

[opencode-agent]

Pull request overview

OpenCode reviewed the current-head bounded evidence and requested changes before merge.

Findings

1. P1 src/sdp/orchestrator.py:70 - Unsafe-function gate bypassable for the named dblink family (dblink_exec/dblink_connect) due to \b word-boundary matching

• Problem: The new unsafe-function loop matches each member of UNSAFE_FUNCTIONS with \b{re.escape(unsafe)}\b. Because '' is a word character, \bdblink\b cannot match dblink_exec, dblink_connect, or dblink_connect_u, and the same suffix mechanism silently misses lo_put/lo_unlink and pg_*-prefixed variants. A payload such as SELECT dblink_exec(conn, stmt) FROM crm — passing both arguments as column values so no string literals, comments, DML keywords, INTO, or and/or appear — passes every other gate (select prefix, FROM-allowlist "crm"), so validate_sql_query returns [] and the browse endpoint returns 200/SUCCEEDED, logging a remote-write primitive as a successful read in the audit trail. This contradicts the PR claim that the documented SELECT-only guarantee is now a real gate rather than a prefix test.
• Root cause: The gate still relies on exact-name word-boundary regex over a fixed set; it cannot match the rest of a function family that the author explicitly intended to block (dblink is listed), and no parser-based or family-aware matching was introduced.
• Fix: Match the family with an optional suffixed member, e.g. change the loop to if re.search(rf"\b{re.escape(unsafe)}(?:_\w*)?\b", lowered): so dblink_exec/dblink_connect/dblink_connect_u, pg_read_file_v2-style variants, and lo_put/lo_unlink are caught (fail-closed for rare identifiers containing such prefixes). Longer term, replace the keyword blocklist with a real SQL parser (e.g. sqlparse) plus a positive allowlist of safe SELECT constructs, and enforce the same validator on every execution path that consumes user SQL.
• Regression test: Extend test_validate_sql_query_blocks_writes_and_volatile_functions with asserts for "SELECT dblink_exec(conn, stmt) FROM crm" and "SELECT dblink_connect(conn) FROM crm" expecting "unsafe_function_call", and run python3 -m pytest tests/test_api.py; add an API-level test mirroring test_browse_query_rejects_select_into_write for the dblink_exec payload asserting 400.
• Suggested diff: posted in this finding's inline review thread.

Summary

PR #32 (head 5438fc7, base 9a7088e) closes the SELECT..INTO and volatile-function bypass of the /browse/query read-only gate, hardens the JWKS URL scheme, and clears the base-branch Semgrep findings. Reviewed changed files: src/sdp/orchestrator.py, src/sdp/authz.py, src/sdp/graph_store.py, src/sdp/observability.py, tests/test_api.py.

Blocking finding P1 — src/sdp/orchestrator.py:70 (unsafe-function loop): the new gate matches each member of UNSAFE_FUNCTIONS with \b{re.escape(unsafe)}\b. For "dblink", the pattern \bdblink\b cannot match "dblink_exec" or "dblink_connect" because the underscore after "dblink" is a word character, so no boundary exists. Payload "SELECT dblink_exec(conn, stmt) FROM crm" (both args passed as column values to avoid the banned string literals) satisfies every other gate: it starts with "select ", has no comments, quotes, and/or, forbidden DML keyword, or INTO, and its FROM table "crm" matches the allowlist — so validate_sql_query returns [] (no warning), /browse/query returns 200, and the audit trail logs a remote-write primitive as a successful read. Same mechanism silently misses lo_put/lo_unlink and pg-suffixed variants. This contradicts the PR claim that the SELECT-only guarantee is now "a real gate, not just a prefix test". Fix direction: match the family with an optional suffix, e.g. rf"\b{re.escape(unsafe)}(?:_\w)?\b", and add regression assertions for dblink_exec/dblink_connect; a real SQL parser + allowlist remains the durable fix. Regression test: extend test_validate_sql_query_blocks_writes_and_volatile_functions and run python3 -m pytest tests/test_api.py.

Falsified probes: (1) file:// JWKS scheme bypass — rejected by test_load_jwks_rejects_non_http_scheme (ValueError raised before urlopen); (2) SELECT..INTO write — rejected with 400 write_operation_not_allowed via test_browse_query_rejects_select_into_write. Clean SELECT on the bound table stays warning-free.

DAG (head flow): flowchart LR A["POST /browse/query"] --> B["validate_sql_query (src/sdp/orchestrator.py)"] B --> C["prefix/comment/literal/keyword gates"] B --> D["\binto\b gate -> 400 write_operation_not_allowed"] B --> E["_UNSAFE_FUNCTIONS \b..\b loop (orchestrator.py:67-70)"] E -. "dblink_exec not matched (no word boundary)" .-> F["SELECT dblink_exec(conn, stmt) FROM crm -> [] warnings -> 200 SUCCEEDED"] D -. "covered by test_browse_query_rejects_select_into_write" .-> G["400"]

Approval sufficiency: not sufficient — central security claim is bypassable (P1 above); one confirmed adversarial probe at src/sdp/orchestrator.py:70. Verification posture: Coverage execution evidence PASS (supported repository test suites passed); no active failed GitHub checks (failed-check-evidence.md reports none); no unresolved review threads at this head (current-head authority). Linter/static: base Semgrep findings (dynamic-urllib-use-detected x2, sqlalchemy-execute-raw-query) cleared via targeted nosemgrep comments; defensible for authz (real scheme hardening) and for observability/graph_store (scheme-validated sink URL, parameterized cypher execute). TDD/regression: three new targeted tests are well-formed but omit the negative case for the bypassable dblink family. Coverage: PASS — coverage_commands python3 -m coverage run -m pytest tests && python3 -m coverage report --show-missing --fail-under=100; Coverage decision PASS. Docstring coverage: no configured docstring gate in review execution contracts; advisory. DAG: Mermaid flowchart above reflects current-head (head) changed flow. PoC/execution: no runtime execution receipt exists; bypass established by source trace of the regex at src/sdp/orchestrator.py:70; JWKS/INTO rejections established by the passing tests in the trusted Coverage execution evidence. DDD/domain: query-safety validation stays in the orchestrator domain layer; coherent with existing policy/domain modules. CDD/context: warnings list -> 400 contract preserved (detail.warnings consumed by tests). Similar issues: keyword/prefix blocklist class already seen in base-branch hardening history (cypher/SQL injection neutralization commits 145b8cf, c2632c0); blocklist approach remains heuristic. Claim/concept check: "SELECT-only guarantee as a real gate" claim is not fully satisfied (P1). Standards search: source limitation — no web/network access; PostgreSQL semantics (SELECT INTO = CREATE TABLE AS; volatile/file-access functions; dblink family) evaluated from source and repository history only. Compatibility/convention: no new DB objects, API fields, or routes; new warning codes write_operation_not_allowed and unsafe_function_call follow the existing two-word snake_case convention of the warnings contract. Breaking-change/backcompat: intended — queries previously returning 200 (SELECT..INTO, volatile functions) now 400; consistent with PR intent and tests. Performance: two additional linear regex scans per query; negligible. Developer experience: focused unit tests with clear failure messages; s3:// source -> table mapping demonstrated in tests. User experience: /browse/query now rejects write/side-effect queries with explicit warning codes in detail.warnings; audit events record rejection reasons; no UI surface. Visual/DOM: non-web change; reviewed API/audit interaction surface (browse.query audit events) instead. Accessibility/i18n: not applicable to this backend change; warning strings are English and consistent with existing codes. Supply-chain/license: no new dependencies (urllib.parse is stdlib). Packaging: pyproject test contract python3 -m pytest tests present; no new source surfaces. Security/privacy: P1 blocker above — \b-boundary blocklist misses dblink_exec/dblink_connect/lo_put-style suffixes, allowing a write primitive to pass the read-only gate and be logged as a successful read; JWKS scheme hardening verified as effective.

Adversarial validation

{"status":"failed","probes":[{"path":"src/sdp/orchestrator.py","line":70,"hypothesis":"The new _UNSAFE_FUNCTIONS gate is bypassable for the dblink family it names: a write primitive reachable through a plain SELECT slips through with zero warnings.","attack_or_counterexample":"SELECT dblink_exec(conn, stmt) FROM crm with conn and stmt supplied as table column values (avoids the banned string literals, comments, and/or, DML keywords, and INTO), so every gate is structurally satisfied.","evidence":"Trusted source trace at src/sdp/orchestrator.py:70: the loop matches each unsafe name with rf\"\\b{re.escape(unsafe)}\\b\"; for \"dblink\" the trailing '_' in dblink_exec/dblink_connect is a word character so \\bdblink\\b finds no boundary and no unsafe_function_call warning is emitted; the FROM-allowlist check then resolves \"crm\" to the expected table, so validate_sql_query returns [] — observed no-warning, no-rejection outcome for the counterexample payload. source-line-sha256=ca0926c1ca211605d63948bf7501a1d47b46a44a3a4c97049d66f87b1f90c153","outcome":"confirmed"},{"path":"tests/test_api.py","line":350,"hypothesis":"The JWKS scheme check could be bypassed so urlopen reads a local file (e.g. /etc/passwd) and the JWKS parse succeeds.","attack_or_counterexample":"_load_jwks_from_url(\"file:///etc/passwd\") with SDP_OIDC_JWKS_TIMEOUT_SECONDS unset.","evidence":"Trusted Coverage execution evidence PASS plus the current-head test test_load_jwks_rejects_non_http_scheme at tests/test_api.py:350, which asserts pytest.raises(ValueError, match=\"unsupported SDP_OIDC_JWKS_URL scheme\") — observed rejection with ValueError raised before urlopen executes for the file:// scheme. source-line-sha256=d69378383e8517eaed846258ea8bd1ae5a87a68897beb8077515c02ddaba7070","outcome":"falsified"}],"residual_risk":"Blocklist/regex SQL gates remain heuristic: even after family-aware matching, unlisted functions, future PostgreSQL built-ins, or multi-statement constructs could evade it; the durable fix is parser-based validation with a positive allowlist. This gate also protects only the /browse/query surface — any other execution path must reuse validate_sql_query. dblink is a contrib extension and current browse execution is mock-backed, so real-world write impact depends on backend wiring; the audit-log misclassification (write attempt recorded as succeeded) is already observable."}

• Result: REQUEST_CHANGES

• Reason: The new SQL read-only gate is bypassable for the very function family it names: \b word-boundary matching in the _UNSAFE_FUNCTIONS loop cannot match dblink_exec/dblink_connect (underscore is a word character), so a column-driven write primitive clears every check and validate_sql_query returns []. Add family-aware matching and a regression test.

• Head SHA: 5438fc75ea431d5aed194d16d0153aa6d570353b

• Workflow run: 30684712573

• Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (4 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (4 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test_api.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_api.py"]
  R2 --> V2["targeted test run"]

Loading

[opencode-agent]

Pull request overview

OpenCode reviewed the current-head bounded evidence and requested changes before merge.

Findings

1. P1 src/sdp/orchestrator.py:13 - SQL read-only gate remains bypassable by uncovered volatile/side-effecting function families

• Problem: The new UNSAFE_FUNCTION_PREFIXES tuple only lists dblink, lo_export, lo_import, pg_ls, pg_read_, pg_sleep, pg_write_. PostgreSQL side-effecting families such as pg_terminate_backend and pg_cancel_backend (session DoS), pg_reload_conf, pg_switch_wal, and the large-object families lo_unlink / lo_put / lo_create / lo_open / lo_truncate (object deletion and mutation) are not matched, so 'SELECT pg_terminate_backend(pid_col) FROM crm' and 'SELECT lo_unlink(oid_col) FROM crm' pass validate_sql_query with warnings=[] and reach 200/SUCCEEDED — the exact class of 'volatile functions that slip past the read-only prefix check' this PR claims to close.
• Root cause: Denylist-by-prefix is incomplete by construction; the PR's own comment asserts the documented SELECT-only guarantee 'holds as a real gate', which is false for uncovered families.
• Fix: Either extend the prefix tuple with the remaining side-effecting families, or invert to an allowlist of read-only functions / restrict to pg_proc.provolatile <> 'v' when a real backend executes the query. Minimal change: add pg_terminate_backend, pg_cancel_backend, pg_reload_conf, pg_switch_wal, pg_create_restore_point, pg_export_snapshot, pg_import_snapshot, lo_open, lo_put, lo_truncate, lo_unlink to _UNSAFE_FUNCTION_PREFIXES.
• Regression test: Extend test_validate_sql_query_blocks_writes_and_volatile_functions in tests/test_api.py with pg_terminate_backend, pg_cancel_backend, pg_reload_conf, lo_unlink and lo_put cases asserting 'unsafe_function_call', then run python3 -m pytest tests/test_api.py.
• Suggested diff: posted in this finding's inline review thread.

Summary

REQUEST_CHANGES for PR #32 (fix(security): reject SELECT..INTO / volatile-function bypass of the SQL read-only gate + clear base Semgrep + JWKS scheme hardening). Inspected changed files src/sdp/orchestrator.py, src/sdp/authz.py, src/sdp/graph_store.py, src/sdp/observability.py, tests/test_api.py. Approval sufficiency: NOT met — the read-only gate claim is falsified for uncovered volatile families. Verification posture: Coverage execution evidence reports PASS (supported repository test suites passed); no active failed GitHub Checks at head; no Playwright/E2E receipts (non-web API change). Linter/static: base Semgrep findings cleared via the new JWKS scheme guard plus nosemgrep suppressions on an already-parameterized AGE query (graph_store.py:478) and a config-controlled sink URL (observability.py:139). TDD/regression: three new tests added (JWKS scheme rejection, SELECT..INTO rejection, volatile-family rejection) but only cover the listed prefixes; missing regression cases for uncovered families. Coverage: PASS per Coverage execution evidence. Docstring coverage: advisory/passed per evidence. DAG: source-backed base-to-head flowchart of the validate_sql_query gate (see review body). PoC/execution: no runtime receipts beyond the reported test-suite PASS; evidence is source-trace + test hunk based. DDD/domain: SQL-safety gate lives in orchestrator domain and matches the browse.query audit contract. CDD/context: gate warnings -> 400 'query_safety_validation_failed' audit behavior consistent with tests. Similar issues: prior opencode-agent CHANGES_REQUESTED (2026-08-01, P1 dblink-family bypass at orchestrator.py:70) is fixed at head via prefix-family matching + tests; no unresolved review threads at head; prior coverage-gate CHANGES_REQUESTED is superseded by current PASS. Claim/concept check: the 'SELECT-only guarantee holds as a real gate' claim is FALSE for pg_terminate_backend/pg_cancel_backend/pg_reload_conf/lo_unlink/lo_put/lo_create — confirmed by source trace of _UNSAFE_FUNCTION_PREFIXES (orchestrator.py:13). Standards search: PostgreSQL volatile-function families per pg_proc and the repo's own comment; no external docs consulted (isolated). Compatibility/convention: snake_case prefixes; no new exposed identifiers, no reserved-word or sequential-id issues on changed surfaces. Breaking-change/backcompat: new warnings are additive to the validate_sql_query list; callers already treat non-empty warnings as rejection; no public API shape change. Implementation completeness: gate logic implemented with tests but denylist is incomplete (P1). Performance: regex compiled once at import; negligible per-query cost. Developer experience: non-web; DX surface is the repo-native pytest suite (python3 -m pytest tests). User experience: non-web; UX surface reviewed is the /browse/query 400 warnings contract. Visual/DOM: no web UI change; non-web interaction surface is the API error contract. Accessibility/i18n: no UI surface changed. Supply-chain/license: no new dependencies (stdlib urllib/urlsplit only). Packaging: pyproject requires-python >=3.10; test command python3 -m pytest tests. Security/privacy: P1 — read-only gate bypassable by uncovered volatile/DoS/large-object functions (pg_terminate_backend DoS, lo_unlink large-object deletion); JWKS scheme hardening is sound.

Adversarial validation

{"status":"failed","probes":[{"path":"src/sdp/authz.py","line":119,"hypothesis":"The new JWKS scheme guard is bypassable: a file:// JWKS URL is still fetched (local file disclosure) or the guard rejects a valid https URL.","attack_or_counterexample":"_load_jwks_from_url('file:///etc/passwd') and https URLs under SDP_OIDC_JWKS_URL","evidence":"Coverage execution evidence reports PASS (supported repository test suites passed); the added test test_load_jwks_rejects_non_http_scheme asserts pytest.raises(ValueError, match='unsupported SDP_OIDC_JWKS_URL scheme') for file:///etc/passwd, and the source trace at src/sdp/authz.py:119 shows the urlsplit scheme guard raising before urlopen is reached; source-line-sha256=b37485413f2f6f6ff48a946f7d1e262224db25484c6ad9cfc844f4d00311b809","outcome":"falsified"},{"path":"src/sdp/orchestrator.py","line":13,"hypothesis":"The previously reported dblink-family bypass persists at current head: dblink_exec / dblink_connect_u / pg_sleep_for / pg_ls_waldir / pg_read_binary_file still pass the gate.","attack_or_counterexample":"SELECT dblink_exec(conn_col, cmd_col) FROM crm and the other family-variant queries","evidence":"Source trace at src/sdp/orchestrator.py:13: the compiled pattern \\b(?:dblink|lo_export|lo_import|pg_ls_|pg_read_|pg_sleep|pg_write_)\\w* matches every family variant, and the added test test_validate_sql_query_blocks_writes_and_volatile_functions asserts unsafe_function_call for all five variants plus write_operation_not_allowed for SELECT..INTO, with Coverage execution evidence reporting the repository test suite passed; source-line-sha256=79c1746db7ea93ab930d6705cd7ba1b7e40eb810fcb3810d6f3461caeaa0739a","outcome":"falsified"},{"path":"src/sdp/orchestrator.py","line":13,"hypothesis":"A SELECT calling an uncovered volatile/side-effecting PostgreSQL function still passes validate_sql_query with warnings=[], reaching 200/SUCCEEDED despite the PR's claim that the SELECT-only guarantee now holds as a real gate.","attack_or_counterexample":"SELECT pg_terminate_backend(pid_col) FROM crm (DoS), SELECT pg_cancel_backend(pid_col) FROM crm, SELECT pg_reload_conf() FROM crm, SELECT lo_unlink(oid_col) FROM crm (large-object deletion/data loss), SELECT lo_put(oid_col, 0, data_col) FROM crm","evidence":"Source trace at src/sdp/orchestrator.py:13: _UNSAFE_FUNCTION_PREFIXES contains only dblink, lo_export, lo_import, pg_ls_, pg_read_, pg_sleep, pg_write_; none of these prefixes matches pg_terminate_backend / pg_cancel_backend / pg_reload_conf / lo_unlink / lo_put / lo_create, and none of the other checks (startswith('select'), \\binto\\b, _FORBIDDEN_KEYWORDS, comment/quote/boolean checks) rejects such a query, so validate_sql_query returns warnings=[]; the tests added in this PR cover only the listed prefixes; source-line-sha256=79c1746db7ea93ab930d6705cd7ba1b7e40eb810fcb3810d6f3461caeaa0739a","outcome":"confirmed"}],"residual_risk":"Residual risk is bounded to the heuristic-gate gap already confirmed: other uncovered side-effecting families (pg_execute_server_program, pg_switch_wal, pg_import_snapshot, lo_open/lo_truncate) can still bypass until an allowlist or pg_proc.provolatile-based check is adopted; JWKS hardening and Semgrep suppressions are covered by tests and passing checks; no runtime execution receipts beyond the reported test-suite PASS."}

• Result: REQUEST_CHANGES

• Reason: SQL read-only gate remains bypassable: uncovered volatile/side-effecting PostgreSQL families (pg_terminate_backend, pg_cancel_backend, pg_reload_conf, lo_unlink, lo_put, lo_create) still pass validate_sql_query with warnings=[] even though the PR claims the SELECT-only guarantee now holds as a real gate.

• Head SHA: 0c085543381cfebeb68055a54a5f0cd90cc2258e

• Workflow run: 30834467661

• Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (4 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (4 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test_api.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_api.py"]
  R2 --> V2["targeted test run"]

Loading

[seonghobae]

Addressed the P1 from the 2026-08-03 OpenCode review in f89c2f5: _UNSAFE_FUNCTION_PREFIXES now also covers the session/server-control families (pg_terminate_*, pg_cancel_*, pg_reload_conf, pg_switch_*, pg_create_*/pg_drop_* restore-point + replication-slot, pg_export_snapshot/pg_import_snapshot, pg_promote, pg_rotate_logfile, pg_wal_replay_*) and the remaining large-object mutation family (lo_unlink, lo_put, lo_truncate, lo_open, lo_create). Regression cases added in test_validate_sql_query_blocks_writes_and_volatile_functions for each family; targeted test passes locally.

[opencode-agent]

Pull request overview

OpenCode reviewed the current-head bounded evidence and requested changes before merge.

Findings

1. P1 src/sdp/orchestrator.py:13 - SQL read-only gate still bypassable by quote-free side-effecting function families (nextval/setval, lo_creat/lo_get, pg_advisory_lock)

• Problem: validate_sql_query rejects only the function families enumerated in _UNSAFE_FUNCTION_PREFIXES; reachable PostgreSQL functions outside that list pass with zero warnings, so the documented SELECT-only guarantee claimed by the PR does not hold.
• Root cause: Name-denylisting cannot close PostgreSQL's unbounded function surface; the gate lacks a structural AST/allow-list check and the executing DB role is not restricted.
• Fix: Extend UNSAFE_FUNCTION_PREFIXES with nextval, setval, lo_creat, lo_get, lo_lseek, pg_advisory; move to a parser-based allow-list of permitted read-only expressions; execute governed queries under a SELECT-only DB role with sequence/large-object privileges revoked and a statement_timeout.
• Regression test: python3 -m pytest tests/test_orchestrator.py -k sql, adding assertions that validate_sql_query('select nextval(1) from crm', source_system='crm') and validate_sql_query('select lo_get(16386) from crm', source_system='crm') return non-empty warnings.
• Suggested diff: posted in this finding's inline review thread.

Summary

PR #32 hardens JWKS URL schemes (src/sdp/authz.py), suppresses base Semgrep findings with justified nosemgrep comments (src/sdp/graph_store.py:478, src/sdp/observability.py:139), adds SELECT..INTO rejection and an unsafe-function prefix denylist (src/sdp/orchestrator.py), extends DatasetPatchRequest with schema (src/sdp_core/contracts.py:254), and adds 12 test modules plus fuzz-harness improvements. Coverage execution evidence: PASS (repo suites passed under coverage with --fail-under=100); no failed checks; no unresolved threads. Approval sufficiency: NOT met. Verification posture: source-trace probes only; no execution receipts available. Linter/static: Semgrep suppressions justified; no failed checks. TDD/regression: extensive new tests, but missing regression coverage for uncovered function families. Coverage: PASS per Coverage execution evidence. Docstring coverage: advisory (no docstring_commands configured). DAG: base-to-head gate-flow diagram included. PoC/execution: none (no OPENCODE_EXECUTION_RECEIPT in evidence). DDD/domain: governed-query domain respected. CDD/context: operator-config trust context documented. Similar issues: two prior bot reviews flagged the same bypass class (2026-08-01 dblink �, 2026-08-03 uncovered families); specific dblink example fixed, class remains. Claim/concept check: read-only guarantee claim contradicted by confirmed nextval/lo_get/pg_advisory_lock pass-through. Standards search: N/A. Compatibility/convention: additive optional schema field, no breaking change. Breaking-change/backcompat: additive warnings only. Implementation completeness: all added code implemented; denylist coverage incomplete. Performance: two linear regex scans, negligible. Developer experience: fuzz README reproducer documentation improved. User experience: non-web CLI/API surface, no regression. Visual/DOM: N/A non-web. Accessibility/i18n: no UI change. Supply-chain/license: no dependency changes. Packaging: unchanged. Security/privacy: JWKS scheme guard correct; SQL gate bypass confirmed (blocking).

Adversarial validation

{"status":"failed","probes":[{"path":"src/sdp/orchestrator.py","line":13,"hypothesis":"The read-only gate can still be bypassed by quote-free side-effecting/exfiltrating PostgreSQL functions absent from _UNSAFE_FUNCTION_PREFIXES, so the documented SELECT-only guarantee is not a real gate.","attack_or_counterexample":"SELECT nextval(16385) FROM crm (regclass accepts numeric OIDs; advances a sequence = write), SELECT setval(16385, 1) FROM crm, SELECT lo_get(16386) FROM crm (reads enumerable large-object OIDs beyond the bound table), SELECT lo_creat(0) FROM crm (denylist has lo_create, not lo_creat), SELECT pg_advisory_lock(1) FROM crm (session-block DoS); all quote-free with the bound FROM crm.","evidence":"Trusted source trace at src/sdp/orchestrator.py:13 (the _UNSAFE_FUNCTION_PREFIXES block in the current-head focused hunk): for `select nextval(16385) from crm`, startswith(\"select \") passes, no comment markers, no quotes, no and/or, nextval not in _FORBIDDEN_KEYWORDS, no `\\binto\\b`, and none of the 23 denylist prefixes (dblink, lo_create, lo_export, lo_import, lo_open, lo_put, lo_truncate, lo_unlink, pg_cancel_, pg_create_, pg_drop_, pg_export_snapshot, pg_import_snapshot, pg_ls_, pg_promote, pg_read_, pg_reload_conf, pg_rotate_logfile, pg_sleep, pg_switch_, pg_terminate_, pg_wal_replay_, pg_write_) match nextval/lo_get/lo_creat/pg_advisory_lock; FROM crm equals the expected source table, so warnings == [] and the statement is allowed to execute despite nextval/lo_creat being writes. Outcome: confirmed. source-line-sha256=79c1746db7ea93ab930d6705cd7ba1b7e40eb810fcb3810d6f3461caeaa0739a","outcome":"confirmed"},{"path":"src/sdp/orchestrator.py","line":13,"hypothesis":"Covered function families can be smuggled past the gate via case variation or schema qualification (e.g. PG_SLEEP or pg_catalog.pg_sleep).","attack_or_counterexample":"SELECT PG_SLEEP(5) FROM crm and SELECT pg_catalog.pg_sleep(5) FROM crm, attempting to evade the denylist through case and schema qualification.","evidence":"Trusted source trace at src/sdp/orchestrator.py:13: _UNSAFE_FUNCTION_PATTERN (\\b(?:...|pg_sleep|...)\\w*) is matched against `lowered` text, so PG_SLEEP is normalized to pg_sleep and the word boundary before pg_sleep matches after the '.' in pg_catalog.pg_sleep; the gate appends unsafe_function_call and rejects the query. Hypothesis falsified: covered families cannot be smuggled by casing or qualification. source-line-sha256=79c1746db7ea93ab930d6705cd7ba1b7e40eb810fcb3810d6f3461caeaa0739a","outcome":"falsified"}],"residual_risk":"The denylist approach remains an arms race: PostgreSQL ships additional reachable side-effect/exfil functions beyond the listed families, so the fix should move to a structural allow-list/parser check plus a restricted DB role (SELECT-only privileges, no sequence/large-object USAGE, statement_timeout). tests/fuzz/run_atheris.sh and tests/fuzz/invariants.py hunks were truncated from inlined evidence and not fully source-inspected (source limitation, nothing contradictory found)."}

• Result: REQUEST_CHANGES

• Reason: The read-only SQL gate still passes quote-free side-effecting function calls (nextval/setval sequence writes, lo_get/lo_creat large-object read/write, pg_advisory_lock session DoS) with zero warnings, so the documented SELECT-only guarantee claimed by the PR is not a real gate.

• Head SHA: d5f683e1ba93294890577d90a6c89d9d7b40b6a2

• Workflow run: 30965470286

• Workflow attempt: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (6 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (6 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test (16 files)"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test (16 files)"]
  R2 --> V2["targeted test run"]

Loading

[opencode-agent]

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

• Problem: Failed same-head checks remain for 33b70c148d9bb3884389d7e61ccb5d8c19cfd75c.
• Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
• Fix: Read and fix the failed check logs below, then rerun the current-head checks.
• Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

• Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184250124/job/92884725407)
• Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184249339/job/92884780082)
• Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184249339/job/92884780082)
• trivy-fs check run: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184250124/job/92884725407)

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Docs: sql-readonly-function-admission.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: sql-readonly-function-admission.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file (6 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (6 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (17 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (17 files)"]
  R3 --> V3["targeted test run"]

Loading

[opencode-agent]

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

• Problem: Failed same-head checks remain for 33b70c148d9bb3884389d7e61ccb5d8c19cfd75c.
• Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
• Fix: Read and fix the failed check logs below, then rerun the current-head checks.
• Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

• Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184250124/job/92884725407)
• Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184249339/job/92884780082)
• Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184249339/job/92884780082)
• trivy-fs check run: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31184250124/job/92884725407)

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Docs: sql-readonly-function-admission.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: sql-readonly-function-admission.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file (6 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (6 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (17 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (17 files)"]
  R3 --> V3["targeted test run"]

Loading

[opencode-agent]

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

• Problem: Failed same-head checks remain for 76fcfb69664a2de49754cc3a94ebd69b5e273870.
• Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
• Fix: Read and fix the failed check logs below, then rerun the current-head checks.
• Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

• Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31738831114/job/94576914593)
• trivy-fs check run: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31738831114/job/94576914593)

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Docs: sql-readonly-function-admission.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: sql-readonly-function-admission.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file: orchestrator.py"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: orchestrator.py"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (2 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (2 files)"]
  R3 --> V3["targeted test run"]

Loading

[opencode-agent]

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

• Problem: Failed same-head checks remain for 76fcfb69664a2de49754cc3a94ebd69b5e273870.
• Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
• Fix: Read and fix the failed check logs below, then rerun the current-head checks.
• Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

• Security Scan/trivy-fs: FAILURE (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31738831114/job/94576914593)
• trivy-fs check run: failure (https://github.com/ContextualWisdomLab/semantic-data-portal/actions/runs/31738831114/job/94576914593)

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Docs: sql-readonly-function-admission.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: sql-readonly-function-admission.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file: orchestrator.py"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: orchestrator.py"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (2 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (2 files)"]
  R3 --> V3["targeted test run"]

Loading