Skip to content

fix(security): redact leaked credentials from bridge logs + add secret-scan guards - #8

Merged
broomva merged 1 commit into
masterfrom
security/redact-leaked-postgres-credential
Jul 13, 2026
Merged

fix(security): redact leaked credentials from bridge logs + add secret-scan guards#8
broomva merged 1 commit into
masterfrom
security/redact-leaked-postgres-credential

Conversation

@broomva

@broomva broomva commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

A security researcher (Robin, sec.scan.github@gmail.com) reported a live Neon Postgres connection string committed to a public conversation-bridge transcript. Investigating the full blast radius surfaced a second leaked credential the report didn't mention: a SYMPHONY_API_TOKEN bearer token (13 occurrences across two logs).

Both are scrubbed here and must be treated as compromised → rotated (the values were public in git history; a tree scrub does not undo historical exposure).

Root cause

The conversation bridge (scripts/conversation-history.py) published raw tool-call transcripts to a PUBLIC repo with no secret redaction. Any railway variables set DATABASE_URL="postgres://…" / --token … command was captured verbatim. This violates the repo's own rule (CLAUDE.mdNever commit real tokens or secret values).

What changed

Area Change
Scrub Redacted the credential + token values in docs/conversations/*.md.
Redact-at-source _redact_secrets() scrubs credentials from generated session docs before write — credentialed URIs, sensitive KEY=VALUE, CLI-flag secrets (--token …), and AWS/Anthropic/OpenAI/GitHub/Slack/PEM token shapes. Precise: leaves source code + placeholders untouched.
Pre-commit gate .githooks/pre-commitscripts/secret-scan.sh --staged.
CI gate .github/workflows/secret-scan.yml runs the scanner + a redactor unit test on every push/PR — independent of the Rust build.
Policy SECURITY.md: responsible-disclosure policy + this incident's disclosure log.

Tests / validation

  • scripts/test_secret_redaction.py30 assertions: real secrets in every form (KEY=, KEY="…", export, --token, credentialed URI, known prefixes) are redacted; source code (resolve_env(&x), String::new(), process.env.X, std::env::var(..)) and placeholders ($VAR, …xxxx, change-me, truncated sk-ant-…) are preserved; redaction is idempotent.
  • Two independent mechanisms (Python redactor + shell scanner) cross-agree on the same token shape.
  • Scanner findings never print the raw secret value.
  • Full tracked-tree scan: CLEAN; no real leaked values remain in the tree.

Merge note

This PR touches only docs + scripts + hook + a new workflow — zero Rust. The repo's Check & Lint / Test jobs are already red on master (pre-existing cargo check --workspace compile break, unrelated to this change), so those required checks will fail here for reasons this PR cannot affect or fix. The gate relevant to this change (Secret Scan) passes. Given this is a security remediation, it should merge despite the pre-existing Rust failure.

Follow-up (operational, out of band)

  • Rotate the Neon Postgres credential and the SYMPHONY_API_TOKEN (mandatory — they were public).
  • Optionally purge the values from git history (BFG/git filter-repo); rotation supersedes this since the values are already compromised.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Security

    • Added automated secret scanning for staged changes, pull requests, pushes, and manual runs.
    • Added automatic redaction of credentials in generated conversation records.
    • Added guidance for vulnerability reporting and secure secret handling.
  • Documentation

    • Replaced exposed credentials in existing conversation examples with safe placeholders.
  • Tests

    • Added coverage for secret redaction, including placeholder preservation and repeatability.

…t-scan guards

A security researcher reported a live Neon Postgres connection string committed
to a public conversation-bridge transcript. Investigation found the raw session
dump had also captured a SYMPHONY_API_TOKEN bearer token (13 occurrences across
two logs). The reported credential and the API token are both scrubbed here and
must be treated as compromised (rotated) — the values were public in git history.

Root cause: the conversation bridge (scripts/conversation-history.py) published
raw tool-call transcripts to a PUBLIC repo with no secret redaction, so any
`railway variables set DATABASE_URL="postgres://…"` / `--token …` command was
committed verbatim.

Remediation (removes value + prevents recurrence):
- Redact the credential + token values in docs/conversations/*.md.
- Redact-at-source: _redact_secrets() scrubs credentials from generated session
  docs before they are written (credentialed URIs, sensitive KEY=VALUE, CLI-flag
  secrets, and AWS/Anthropic/OpenAI/GitHub/Slack/PEM token shapes). Precise —
  leaves source code and placeholders untouched.
- Pre-commit gate: .githooks/pre-commit runs scripts/secret-scan.sh --staged.
- CI gate: .github/workflows/secret-scan.yml runs the scanner + a unit test of
  the redactor on every push/PR (independent of the Rust build).
- SECURITY.md: responsible-disclosure policy + this incident's disclosure log.

Validated: scripts/test_secret_redaction.py (30 assertions — real secrets in
every form redacted; code/placeholders preserved; idempotent). The scanner and
the redactor agree on the same token shape, and the scanner never prints a raw
secret in its findings.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds secret redaction to generated conversation documents, a repository scanner, local and CI enforcement gates, sanitized transcript examples, and a security policy covering reporting and credential handling.

Changes

Secret protection

Layer / File(s) Summary
Transcript redaction and validation
scripts/conversation-history.py, scripts/test_secret_redaction.py
Generated markdown is sanitized for credential-like values, with tests covering removal, preservation, and idempotency.
Repository secret scanner
scripts/secret-scan.sh
A Bash scanner checks staged, tracked, or specified files, filters allowed placeholders, redacts findings, and fails when potential credentials are detected.
Local and CI enforcement
.githooks/pre-commit, .github/workflows/secret-scan.yml, .gitignore
Pre-commit and GitHub Actions run the scanner and redaction tests; Python cache directories are ignored.
Security policy and sanitized records
SECURITY.md, docs/conversations/*.md
Security reporting and secret-handling rules are documented, and exposed credentials in conversation examples are replaced with placeholders.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Developer
  participant PreCommitHook
  participant SecretScanner
  participant GitHubActions
  Developer->>PreCommitHook: commit staged changes
  PreCommitHook->>SecretScanner: scan staged files
  GitHubActions->>SecretScanner: scan repository files
  GitHubActions->>RedactionTests: run redaction tests
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main security fix: redacting leaked credentials and adding secret-scan safeguards.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/redact-leaked-postgres-credential

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

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 4

🧹 Nitpick comments (1)
scripts/secret-scan.sh (1)

34-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Align scanner case-sensitivity with the Python redactor.

The bash scanner uses case-sensitive grep -E while the Python redactor (_KV_SECRET, _FLAG_SECRET, _URI_CRED) uses (?i. A lowercase assignment like api_token=secretvalue1234567890 would be redacted by the bridge but missed by the scanner, weakening the defense-in-depth contract.

Adding -i to both the pattern and ALLOW grep invocations would align the two layers.

♻️ Proposed fix
-    grep -nE "$pat" "$f" 2>/dev/null | grep -vE "$ALLOW" | while IFS=: read -r ln rest; do
+    grep -inE "$pat" "$f" 2>/dev/null | grep -ivE "$ALLOW" | while IFS=: read -r ln rest; do

Also applies to: 37-50

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/secret-scan.sh` at line 34, Make the Bash scanner case-insensitive to
match the Python redactor: add the case-insensitive grep option to the KV secret
scan and the related ALLOW-list grep invocations covering lines 37–50. Ensure
lowercase secret keys such as api_token are detected while preserving the
existing patterns and filtering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/secret-scan.yml:
- Line 26: Update the actions/checkout@v4 step in the secret-scan workflow to
set persist-credentials to false, preventing the checkout action from storing
the GITHUB_TOKEN in the repository configuration.

In `@docs/conversations/session-2026-03-17-635dce40.md`:
- Around line 3027-3035: Remove the blank lines within the blockquote containing
the daemon status and refresh tool call, keeping the quoted Markdown contiguous
or prefixing any intentional blank lines with “>” so the blockquote remains
valid.

In `@SECURITY.md`:
- Around line 8-10: Update the security reporting guidance in SECURITY.md to
provide a dedicated, monitored security email address, or remove the unstable
“commit author / GitHub profile” fallback while retaining the preferred GitHub
private advisory route.
- Around line 17-19: Update the related policy reference in the SECURITY.md note
from the plain `CLAUDE.md` text to the repository-standard `[[CLAUDE.md]]`
wikilink, preserving the surrounding Safety Rules reference and documentation
content.

---

Nitpick comments:
In `@scripts/secret-scan.sh`:
- Line 34: Make the Bash scanner case-insensitive to match the Python redactor:
add the case-insensitive grep option to the KV secret scan and the related
ALLOW-list grep invocations covering lines 37–50. Ensure lowercase secret keys
such as api_token are detected while preserving the existing patterns and
filtering behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b34db02d-a817-4aff-b57a-b209c95b5dfb

📥 Commits

Reviewing files that changed from the base of the PR and between d1c0889 and 5f76670.

📒 Files selected for processing (9)
  • .githooks/pre-commit
  • .github/workflows/secret-scan.yml
  • .gitignore
  • SECURITY.md
  • docs/conversations/session-2026-03-17-635dce40.md
  • docs/conversations/session-2026-03-18-72911522.md
  • scripts/conversation-history.py
  • scripts/secret-scan.sh
  • scripts/test_secret_redaction.py

name: Secret Scan
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Set persist-credentials: false on checkout.

By default actions/checkout@v4 persists the GITHUB_TOKEN in .git/config. Since this workflow runs on pull_request (including fork PRs) and executes repo-provided scripts, a malicious PR could read the persisted credential. The fix is a one-line addition.

🔒 Proposed fix
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- uses: actions/checkout@v4
- uses: actions/checkout@v4
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 26-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/secret-scan.yml at line 26, Update the actions/checkout@v4
step in the secret-scan workflow to set persist-credentials to false, preventing
the checkout action from storing the GITHUB_TOKEN in the repository
configuration.

Source: Linters/SAST tools

Comment on lines +3027 to +3035
>> `symphony --host symphony-production-0eaf.up.railway.app --token [REDACTED_SECRET] status 2>&1`

> [!info] **Assistant**
> Daemon is running but no issues dispatching. The issue is likely that it's finding 40 candidates but none are being dispatched — probably because the agent command was failing before you logged in, and now the retry backoff is waiting. Let me trigger a refresh and check the logs.

> [!example] Tool Calls
>> [!note] **Bash** — Trigger immediate poll
>> *Trigger immediate poll*
>> `symphony --host symphony-production-0eaf.up.railway.app --token f68ca32dbf29a16e7ade0f1a3b7022944103d678adb85b1c615856753884d343 refresh 2>&1`
>> `symphony --host symphony-production-0eaf.up.railway.app --token [REDACTED_SECRET] refresh 2>&1`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove blank lines inside the blockquote.

Markdownlint reports MD028 at Lines 3028 and 3031. Keep the blockquote contiguous, or prefix blank lines with >.

🧰 Tools
🪛 markdownlint-cli2 (0.23.0)

[warning] 3028-3028: Blank line inside blockquote

(MD028, no-blanks-blockquote)


[warning] 3031-3031: Blank line inside blockquote

(MD028, no-blanks-blockquote)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/conversations/session-2026-03-17-635dce40.md` around lines 3027 - 3035,
Remove the blank lines within the blockquote containing the daemon status and
refresh tool call, keeping the quoted Markdown contiguous or prefixing any
intentional blank lines with “>” so the blockquote remains valid.

Source: Linters/SAST tools

Comment thread SECURITY.md
Comment on lines +8 to +10
- **Preferred:** GitHub → *Security* tab → *Report a vulnerability* (private
advisory) on this repository.
- **Email:** the repository owner (see the commit author / GitHub profile).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Provide a concrete private reporting contact.

“See the commit author / GitHub profile” is not a stable or guaranteed monitored channel. Publish a dedicated security email, or remove this fallback.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SECURITY.md` around lines 8 - 10, Update the security reporting guidance in
SECURITY.md to provide a dedicated, monitored security email address, or remove
the unstable “commit author / GitHub profile” fallback while retaining the
preferred GitHub private advisory route.

Comment thread SECURITY.md
Comment on lines +17 to +19
Symphony's own rule (see `CLAUDE.md` → *Safety Rules*) is: **never commit real
tokens or secret values**. Credentials belong in environment variables or a
secret manager, never in source, config, or documentation.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Link the related policy with a wikilink.

Change CLAUDE.md to [[CLAUDE.md]] so this documentation note participates in the repository’s knowledge graph.

As per coding guidelines: **/*.md: Use [[wikilinks]] to connect related notes in documentation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SECURITY.md` around lines 17 - 19, Update the related policy reference in the
SECURITY.md note from the plain `CLAUDE.md` text to the repository-standard
`[[CLAUDE.md]]` wikilink, preserving the surrounding Safety Rules reference and
documentation content.

Source: Coding guidelines

@broomva
broomva merged commit 7ac6f3c into master Jul 13, 2026
7 of 9 checks passed
@broomva
broomva deleted the security/redact-leaked-postgres-credential branch July 13, 2026 18:47
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