Skip to content

fix(security): remediate 2026-07-12 re-audit findings#626

Merged
SoundMindsAI merged 2 commits into
mainfrom
security/audit-2026-07-12-batch
Jul 12, 2026
Merged

fix(security): remediate 2026-07-12 re-audit findings#626
SoundMindsAI merged 2 commits into
mainfrom
security/audit-2026-07-12-batch

Conversation

@SoundMindsAI

Copy link
Copy Markdown
Owner

Summary

Batch remediation of the second security re-audit (six parallel agents). The re-audit verified the #623 hardening and found three of its fixes were incomplete plus one pre-existing agent-gate weakness. No Critical findings.

Sev Finding Fix
HIGH SSRF: #623's reuse-path guard missed build_adapter, so 5 worker/dispatch paths (trials, baseline, judgments, judgments_ubi, agent dispatch) — the longest-lived egress — still bypassed it New async build_adapter_checked chokepoint; all 5 routed through it
MED Solr local-params: #623 neutralized only q; query_text could reach fq/bf/boost/pf Neutralize at the source (render entry) — safe wherever a template places it
MED Confirmation gate bound to tool name not args → one "yes" authorized unlimited invocations vs arbitrary targets (F1) One-mutation-per-turn invariant
MED A negated mention ("I will not cancel_study") counted as a proposal (F2) Conservative 3-word negation window
LOW Apostrophe negation defeated by unicode smart quotes (F3) Fold U+2019 etc. to ASCII
LOW config_diff validator skipped rich-form declared_params Handle {"type": ...} dict shape
LOW ` tojson` guard bypassable by a trailing filter
LOW template.name unrestricted → params-file path relocation Path-safe pattern
LOW Log scrubber lacked DSN password coverage Redact scheme://user:pass@host
LOW Smoke job could leak OPENAI_API_KEY_TEST to fork PRs Fork-PR if: guard
LOW Frontend: digest-panel inlined disallowed-elements; noisy report-only connect-src Shared constant; drop connect-src

Test plan

  • make test-unit — 2909 passed (~30 new regression tests)
  • make lint && make typecheck (mypy + tsc clean)
  • cd ui && pnpm test (vitest) — pass
  • bash scripts/regen-generated-artifacts.shui/openapi.json refreshed
  • DB-backed contract/integration in CI

Notes

  • Cross-model review: Opus self-review (GPT-5.5 unreachable); Gemini is the live cross-family gate.
  • Repo-settings items handled separately (not code): enabling the CodeQL "code-scanning results" merge rule, and PR-approval count (deferred until a second collaborator — would block solo self-merge today).

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements several security hardening measures based on a security audit, including SSRF protection for cluster connections, Solr local-params neutralization, unicode apostrophe folding for negation checks, a limit of one mutating tool dispatch per turn, path-safe validation for query template names, and DSN password redaction. The reviewer identified a high-severity issue in backend/app/agent/orchestrator.py where the negation check on assistant responses can be bypassed using unicode smart quotes because the text is not folded before matching.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread backend/app/agent/orchestrator.py
Batch fix for the second security re-audit, which verified the #623
hardening and found three of its fixes were incomplete plus one
pre-existing agent-gate weakness. No Critical findings.

HIGH
- SSRF: #623 added the reuse-path guard to acquire_adapter/reprobe/
  get_or_probe_health but NOT to the 5 worker/dispatch paths (trials,
  baseline, judgments, judgments_ubi, agent dispatch) — the longest-lived
  cluster-egress paths — which call build_adapter directly. Each now
  `await assert_base_url_allowed(cluster.base_url)` before build_adapter,
  so the DNS-rebinding re-check fires on every connection (no-op unless
  RELYLOOP_ALLOW_PRIVATE_CLUSTERS=False).

MEDIUM
- Solr local-params: #623 neutralized only `q`; query_text could reach
  fq/bf/boost/pf via a template. Now neutralized at the source (render
  entry) so it's safe wherever a template places it.
- Confirmation gate F1: bound to tool NAME not args → one "yes"
  authorized unlimited invocations vs arbitrary targets. New
  one-mutation-per-turn invariant.
- Confirmation gate F2: a NEGATED mention ("I will not cancel_study") no
  longer counts as a proposal (conservative 3-word negation window).

LOW
- Apostrophe (F3): fold unicode smart apostrophes (U+2019 etc.).
- config_diff value validator handles rich-form declared_params.
- tojson guard requires tojson to be the OUTERMOST filter.
- template.name path-safe pattern (interpolated into params-file path).
- Log scrubber redacts scheme://user:password@host DSN passwords.
- CI: fork-PR guard on the smoke job (OPENAI_API_KEY_TEST).
- Frontend: digest-panel uses shared MARKDOWN_DISALLOWED_ELEMENTS;
  report-only connect-src dropped to avoid per-API-call noise.

~30 new regression tests. Regenerates ui/openapi.json (template.name
pattern). Repo-settings items (CodeQL results merge rule; PR-approval
count) handled separately.

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
@SoundMindsAI SoundMindsAI force-pushed the security/audit-2026-07-12-batch branch from b2a37cf to 48b6ef3 Compare July 12, 2026 02:18
_tool_mention_is_negated uses ASCII apostrophe forms (won'?t), but
assistant_lower wasn't apostrophe-folded, so a smart-quote 'won't' could
bypass the F2 negation check. Fold assistant_lower in _is_authorized_mutation
so both tool-matching and negation use the ASCII form. Accepted from Gemini.

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
@SoundMindsAI

Copy link
Copy Markdown
Owner Author

Review adjudication (Gemini Code Assist)

# Sev Location Verdict Notes
1 Medium backend/app/agent/orchestrator.py:203 Accepted Real bug (same class as F3): assistant_lower wasn't apostrophe-folded before the negation regex, so a smart-quote won't could bypass the F2 negation check. Fixed — assistant_lower is now folded in _is_authorized_mutation so both tool-matching and negation use the ASCII form. Regression test added.

Applied (1) / Rejected (0) / Deferred (0).

@SoundMindsAI SoundMindsAI merged commit 7be01e0 into main Jul 12, 2026
21 checks passed
@SoundMindsAI SoundMindsAI deleted the security/audit-2026-07-12-batch branch July 12, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant