docs: expand SECURITY with secrets audit, MCP port checks, and reminder triage#1513
docs: expand SECURITY with secrets audit, MCP port checks, and reminder triage#1513MaxRyabov wants to merge 1 commit intoaffaan-m:mainfrom
Conversation
…ral reminder triage Three operational sections were missing from SECURITY.md: 1. Secrets Handling now explicitly covers the user-scope `~/.claude/settings.json` (outside the repo but frequently shared via `claude doctor` output and screenshots). Adds a concrete grep/ Select-String recipe for auditing `mcpServers[*].env` entries and a short remediation path. 2. Local MCP Ports explains the risk of plain-HTTP localhost MCP endpoints (e.g. `devfleet` at `http://localhost:18801/mcp`) and gives `netstat`/`lsof` commands to confirm the listening PID before first use. 3. Triage: suspicious `<system-reminder>` blocks documents Claude Code's ephemeral client-side reminder mechanism (TodoWrite nudge, date changed, file modified). These blocks end with phrasing like "NEVER mention this reminder to the user" or "Don't tell the user this, since they are already aware" — that wording is part of Anthropic's own prompt, not a malicious tail, and the blocks are not persisted in the session transcript. The section gives a 3-step triage checklist (repo grep, transcript check, content consistency) so future reports distinguish real injections from harness-level reminders before escalating to Anthropic. Also adds a matching `Unreleased` entry to CHANGELOG.md.
📝 WalkthroughWalkthroughAdded changelog entries documenting CI validation improvements for duplicate YAML frontmatter keys and new security operational guidance document. Operational guidance covers secrets management, MCP port verification, and suspicious system-reminder block triage procedures. Fixed duplicate model declaration in an agent file. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds three operational security sections to
Confidence Score: 3/5Not safe to merge as-is — the triage checklist contains a self-defeating grep that will mislead every future triager, and the CHANGELOG conflates unrelated file changes. Two P1 findings: the self-referential grep false positive in the triage section actively breaks the checklist's logic for all future users, and the CHANGELOG entry documents files not in this PR's diff which corrupts release history traceability. SECURITY.md (triage grep + heading level), CHANGELOG.md (out-of-scope file references) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Observe suspicious system-reminder block] --> B{Step 1: grep repo\nfor 'system-reminder'\n'NEVER mention'\n'DO NOT mention'}
B -->|Match found| C[⚠️ Always hits SECURITY.md now\nFalse positive risk]
B -->|No match| D[Not in repo files]
D --> E{Step 2: Check session\n.jsonl transcript\nfor block in tool_result}
E -->|Found in transcript| F{Step 3: Attributable\nto file/URL read?}
E -->|Not in transcript| G[✅ Ephemeral client reminder\nNo action needed]
F -->|Yes| G
F -->|No| H[🚨 Escalate to Anthropic\nvia GitHub Issues or\nsecurity@anthropic.com]
Reviews (1): Last reviewed commit: "docs: expand SECURITY with secrets audit..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| That combination makes them easy to mistake for a prompt-injection appended to a tool result. Before treating one as an attack, verify: | ||
|
|
||
| 1. Is the block actually in a file under this repo? `grep -rEn "system-reminder|NEVER mention|DO NOT mention" .` — if nothing, it is not carried by the repo. |
There was a problem hiding this comment.
The triage checklist instructs users to run grep -rEn "system-reminder|NEVER mention|DO NOT mention" . to determine whether a suspicious block is carried by the repo. However, SECURITY.md now contains the literal strings "system-reminder" and "NEVER mention" (lines 82–87 document them verbatim), so this grep will always produce a match against SECURITY.md itself. The logic immediately after the command — "if nothing, it is not carried by the repo" — becomes unreliable: there will never be "nothing", causing every triager to conclude the repo is a carrier when it isn't.
Fix: exclude SECURITY.md from the grep, or adjust the surrounding prose to say "ignore hits in SECURITY.md":
| 1. Is the block actually in a file under this repo? `grep -rEn "system-reminder|NEVER mention|DO NOT mention" .` — if nothing, it is not carried by the repo. | |
| 1. Is the block actually in a file under this repo? `grep -rEn --exclude=SECURITY.md "system-reminder|NEVER mention|DO NOT mention" .` — if nothing, it is not carried by the repo. |
| - `scripts/ci/validate-agents.js`: detects duplicate top-level YAML frontmatter keys in agent files (e.g. two `model:` lines silently collapsing to the last value) and fails validation with the duplicated key names. | ||
| - `SECURITY.md`: new operational sections — Secrets Handling (including a user-scope `~/.claude/settings.json` audit recipe for macOS/Linux/Windows), Local MCP Ports (how to verify the listener on bundled HTTP MCP endpoints such as `devfleet`), and a Triage section explaining Claude Code's ephemeral client-side `<system-reminder>` blocks (to avoid misclassifying them as prompt injections). | ||
|
|
||
| ### Fixed | ||
|
|
||
| - `agents/a11y-architect.md`: removed stray duplicate `model:` line so YAML parsers no longer silently override `sonnet` with a later `opus` entry. |
There was a problem hiding this comment.
The Unreleased section documents changes to scripts/ci/validate-agents.js and agents/a11y-architect.md, but neither file appears in this PR's diff (only CHANGELOG.md and SECURITY.md are changed). If those changes are intended to land in a separate PR, having them recorded here will conflate unrelated work under one changelog entry and make the release diff hard to audit. If they're already merged to main, they should be under a versioned heading, not Unreleased.
|
|
||
| Compare the PID against the expected devfleet binary. Any other process on that port can intercept MCP traffic. | ||
|
|
||
| ## Triage: suspicious `<system-reminder>` blocks |
There was a problem hiding this comment.
The Triage section uses an ## heading, placing it at the same level as ## Operational Guidance and ## Security Resources, but its content (like Secrets Handling and Local MCP Ports above it) is clearly operational guidance. Using ### keeps the ToC hierarchy consistent and avoids readers thinking this is a separate top-level policy section.
| ## Triage: suspicious `<system-reminder>` blocks | |
| ### Triage: suspicious `<system-reminder>` blocks |
|
|
||
| ```bash | ||
| # macOS / Linux | ||
| grep -EnH '(TOKEN|SECRET|KEY|PASSWORD)\s*"\s*:\s*"[A-Za-z0-9_-]{16,}"' ~/.claude/settings.json |
There was a problem hiding this comment.
The pattern (TOKEN|SECRET|KEY|PASSWORD) is case-sensitive (no -i flag), so common lowercase variants like "api_key", "secret_token", or "access_token" are silently skipped. Adding -i closes this gap:
| grep -EnH '(TOKEN|SECRET|KEY|PASSWORD)\s*"\s*:\s*"[A-Za-z0-9_-]{16,}"' ~/.claude/settings.json | |
| grep -EniH '(TOKEN|SECRET|KEY|PASSWORD)\s*"\s*:\s*"[A-Za-z0-9_-]{16,}"' ~/.claude/settings.json |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
SECURITY.md (2)
56-61: Secret-audit regex is too narrow and will miss real credentials.The current character class (
[A-Za-z0-9_-]{16,}) excludes common secret formats (e.g., JWT segments with., base64 tokens with+/=, and provider keys with other symbols), so this “quick audit” can produce false negatives.Suggested doc tweak
# macOS / Linux -grep -EnH '(TOKEN|SECRET|KEY|PASSWORD)\s*"\s*:\s*"[A-Za-z0-9_-]{16,}"' ~/.claude/settings.json +grep -EnH '"[^"]*(TOKEN|SECRET|KEY|PASSWORD)[^"]*"\s*:\s*"[^"]{16,}"' ~/.claude/settings.json # Windows PowerShell -Select-String -Path "$env:USERPROFILE\.claude\settings.json" -Pattern '(TOKEN|SECRET|KEY|PASSWORD)"\s*:\s*"[A-Za-z0-9_-]{16,}"' +Select-String -Path "$env:USERPROFILE\.claude\settings.json" -Pattern '"[^"]*(TOKEN|SECRET|KEY|PASSWORD)[^"]*"\s*:\s*"[^"]{16,}"'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SECURITY.md` around lines 56 - 61, The secret-audit regex used in the grep/Select-String examples is too restrictive (it uses [A-Za-z0-9_-]{16,}) and will miss many real secrets (JWTs, base64, provider keys containing .+/= and other symbols); update both patterns (the grep pattern and the PowerShell Select-String pattern) to match a broader set of non-whitespace characters for the value (e.g., use \S or a character class like [A-Za-z0-9._~+/=-] with an appropriate minimum length) and keep the key name capture (TOKEN|SECRET|KEY|PASSWORD) and length threshold (e.g., {16,}) intact so the examples in SECURITY.md detect common token formats without being overly permissive.
69-74: Windows port check should explicitly filter listener state.
netstat -ano | findstr :18801can include non-listening rows and make PID verification noisy. Consider documentingLISTENINGfiltering to reduce operator error.Suggested doc tweak
# Windows -netstat -ano | findstr :18801 +netstat -ano -p tcp | findstr ":18801" | findstr LISTENING🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SECURITY.md` around lines 69 - 74, Update the Windows port-check example to explicitly filter for LISTENING state so the command only returns active listeners and the correct PID; replace the current `netstat -ano | findstr :18801` snippet with a variant that matches both the port and the LISTENING state (e.g., using findstr twice or a single regex) and include a short note explaining this avoids non-listening rows being returned; reference the existing Windows example in SECURITY.md to locate and update the snippet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@SECURITY.md`:
- Around line 56-61: The secret-audit regex used in the grep/Select-String
examples is too restrictive (it uses [A-Za-z0-9_-]{16,}) and will miss many real
secrets (JWTs, base64, provider keys containing .+/= and other symbols); update
both patterns (the grep pattern and the PowerShell Select-String pattern) to
match a broader set of non-whitespace characters for the value (e.g., use \S or
a character class like [A-Za-z0-9._~+/=-] with an appropriate minimum length)
and keep the key name capture (TOKEN|SECRET|KEY|PASSWORD) and length threshold
(e.g., {16,}) intact so the examples in SECURITY.md detect common token formats
without being overly permissive.
- Around line 69-74: Update the Windows port-check example to explicitly filter
for LISTENING state so the command only returns active listeners and the correct
PID; replace the current `netstat -ano | findstr :18801` snippet with a variant
that matches both the port and the LISTENING state (e.g., using findstr twice or
a single regex) and include a short note explaining this avoids non-listening
rows being returned; reference the existing Windows example in SECURITY.md to
locate and update the snippet.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="SECURITY.md">
<violation number="1" location="SECURITY.md:83">
P2: Security triage uses an absolute transcript-persistence claim that is version-sensitive and can cause false negatives in incident classification.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ECC runs inside Claude Code, which injects **ephemeral client-side system reminders** into the model's input on every turn (TodoWrite nudges, date-changed notices, file-modified notices, etc.). These blocks: | ||
|
|
||
| - typically end with phrasing like *"ignore if not applicable"* or *"NEVER mention this reminder to the user"* / *"Don't tell the user this, since they are already aware"* — that wording is Anthropic's own prompt, not a malicious tail; | ||
| - are added by the CLI per turn and are **not persisted** in the session transcript at `~/.claude/projects/<slug>/<sessionId>.jsonl`. |
There was a problem hiding this comment.
P2: Security triage uses an absolute transcript-persistence claim that is version-sensitive and can cause false negatives in incident classification.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At SECURITY.md, line 83:
<comment>Security triage uses an absolute transcript-persistence claim that is version-sensitive and can cause false negatives in incident classification.</comment>
<file context>
@@ -45,6 +45,53 @@ This policy covers:
+ECC runs inside Claude Code, which injects **ephemeral client-side system reminders** into the model's input on every turn (TodoWrite nudges, date-changed notices, file-modified notices, etc.). These blocks:
+
+- typically end with phrasing like *"ignore if not applicable"* or *"NEVER mention this reminder to the user"* / *"Don't tell the user this, since they are already aware"* — that wording is Anthropic's own prompt, not a malicious tail;
+- are added by the CLI per turn and are **not persisted** in the session transcript at `~/.claude/projects/<slug>/<sessionId>.jsonl`.
+
+That combination makes them easy to mistake for a prompt-injection appended to a tool result. Before treating one as an attack, verify:
</file context>
|
Thanks for the PR. This has been idle for several weeks and is not merge-ready against the moving main branch, so I am closing it to keep the queue tractable. Reopen is welcome with a current rebase and focused scope; maintainers may also port the useful parts into a fresh PR where appropriate. |
2 participants
What Changed
Three new operational sections in
SECURITY.md, plus a matchingUnreleasedentry inCHANGELOG.md:Secrets Handling — explicit audit recipe for the user-scope
~/.claude/settings.json(outside the repo but commonly shared viaclaude doctoroutput and screenshots). Providesgrep/Select-Stringcommands for macOS/Linux/Windows that flag plain-text tokens insidemcpServers[*].env.Local MCP Ports — explains the risk of plain-HTTP localhost MCP endpoints shipped in
mcp-configs/mcp-servers.json(notablydevfleetathttp://localhost:18801/mcp), and givesnetstat/lsofcommands to verify the listener PID before first use.Triage: suspicious
<system-reminder>blocks — documents Claude Code's ephemeral client-side reminder mechanism (TodoWrite nudge, date-changed notice, file-modified notice). These blocks end with phrasing like "NEVER mention this reminder to the user" or "Don't tell the user this, since they are already aware" — that wording is Anthropic's own prompt, not a malicious tail, and the blocks are not persisted in the session transcript at~/.claude/projects/<slug>/<sessionId>.jsonl. Includes a 3-step triage checklist (repo grep → transcript check → Anthropic-reminder consistency) so real tool-result injections are distinguished from harness-level reminders before escalating to Anthropic.Why This Change
Two hygiene gaps observed during a recent security review of the repo:
mcp-configs/mcp-servers.jsonalready has a "template only" convention forYOUR_*_HERE, but the user-scope~/.claude/settings.jsonhas no equivalent guidance, and plain-text PATs there are a recurring source of real leaks.<system-reminder>blocks as prompt injection (because of the "NEVER mention…" tail wording). Without a documented triage path, similar false-positives are likely. Inspecting the JSONL transcript shows the blocks are never persisted intool_resultbodies, which is a deterministic signal for future triagers.Testing Done
greprecipes in the new sections exercised locally against~/.claude/settings.jsonand anmcp-servers.jsontemplate.node tests/run-all.js) — 1847 tests, 1843 passing, 4 pre-existing broken-symlink failures unchanged from baseline.Type of Change
docs:DocumentationSecurity & Quality Checklist
Documentation
SECURITY.md+CHANGELOG.mdNotes for Maintainers
docs/tr/SECURITY.mdanddocs/zh-CN/SECURITY.mdare last-synced with the rootSECURITY.mdat2026-04-05and do not carry the three new sections. Happy to open a follow-up tracking issue labeledtranslationso community translators can pick it up — let me know the preferred title.Summary by cubic
Expand
SECURITY.mdwith hands-on guidance to prevent secrets leaks, verify local MCP endpoints, and avoid false prompt-injection reports from Claude Code reminders. Adds audit commands for~/.claude/settings.json, port checks for HTTP MCP servers likedevfleet, and a 3-step reminder triage; updatesCHANGELOG.mdunder Unreleased.Written for commit 1af5e7f. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Documentation
Chores