From 442a324abb1cf9a61ac0990b865d7d86f03f50fd Mon Sep 17 00:00:00 2001 From: Angel Date: Sat, 12 Sep 2026 17:06:13 +0200 Subject: [PATCH 1/2] fix: secret scan missed every prefixed secret name (db_password, STRIPE_SECRET) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Hardcoded secret assignment" rule wrapped its keyword alternation in `\b`. `_` is a word character, so `\b` never fires between an underscore and a letter and every prefixed spelling slipped past a BLOCKING guard: HIT password = "..." MISS db_password = "..." HIT api_key = "..." MISS my_api_key = "..." HIT client_secret: "..." MISS STRIPE_SECRET = "..." MISS aws_secret_access_key = "..." The MISS column is the dominant real-world spelling — snake_case and SCREAMING_SNAKE in Python, YAML and .env content, which is exactly what gets pasted into a learning. `client_secret` hit only because `client[_-]?secret` is spelled out as its own alternative, which disguised the gap: a spot check of the obvious names passes. Found by planting an `aws_secret_access_key` canary in the store right after installing the pre-push hook; the scan exited 0. Replace the `\b` anchors with an explicit non-alphanumeric left boundary plus flat affix character classes (not nested quantifiers, so backtracking stays linear — a pathological 800-char line searches in ~1ms). The value stays capture group 1, which `_is_placeholder` depends on. Verified the widening does not cost the placeholder filter its job: `db_password = changeme` and `my_api_key = "your_key_here"` are still clean, and short/non-secret values (`password: x`, `secret_name = db`) still do not match. Also gitignore this repo's own `learnings/`: they are local dev notes, not content shipped to users of the plugin. Co-Authored-By: Claude Opus 5 --- .gitignore | 5 +++++ plugin/scripts/scan_secrets.py | 14 +++++++++++--- tests/test_scan_secrets.py | 20 ++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index db2eeae..880afb9 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,8 @@ __pycache__/ *.pyc .DS_Store Thumbs.db + +# This repo's own learnings are local dev notes, not something shipped to users +# of the plugin. Un-ignoring them turns them into published content — see the +# security note in learnings/README.md before doing so. +learnings/ diff --git a/plugin/scripts/scan_secrets.py b/plugin/scripts/scan_secrets.py index c78b159..effde5e 100644 --- a/plugin/scripts/scan_secrets.py +++ b/plugin/scripts/scan_secrets.py @@ -51,10 +51,18 @@ ("JWT", r"\beyJ[A-Za-z0-9_\-]+\.[A-Za-z0-9_\-]+\.[A-Za-z0-9_\-]{10,}\b"), ("Bearer token", r"(?i)\bbearer\s+[A-Za-z0-9_\-\.=]{20,}"), ("Credentials in URL", r"\b[a-z][a-z0-9+.\-]*://[^\s:@/]+:([^\s:@/]{3,})@[^\s/]+"), - # secret-ish assignment with a real-looking value (placeholders filtered below) + # secret-ish assignment with a real-looking value (placeholders filtered below). + # The keyword is deliberately NOT wrapped in `\b`: `_` is a word character, so + # `\b` never fires between an underscore and a letter, and `db_password`, + # `STRIPE_SECRET` and `aws_secret_access_key` — the dominant real-world + # spellings — all slipped through. Affixes are flat character classes rather + # than nested quantifiers, so backtracking stays linear. + # The value must stay capture group 1: `_is_placeholder` reads `group(1)`. ("Hardcoded secret assignment", - r"(?i)\b(?:password|passwd|pwd|secret|api[_-]?key|access[_-]?token|" - r"auth[_-]?token|client[_-]?secret|private[_-]?key)\b\s*[:=]\s*" + r"(?i)(?:^|[^A-Za-z0-9])[A-Za-z0-9_.-]{0,40}" + r"(?:password|passwd|pwd|secret|api[_-]?key|access[_-]?token|" + r"auth[_-]?token|client[_-]?secret|private[_-]?key)" + r"[A-Za-z0-9_.-]{0,40}\s*[:=]\s*" r"['\"]?([^\s'\"]{6,})['\"]?"), ] RULES = [(name, re.compile(pat)) for name, pat in _RULES] diff --git a/tests/test_scan_secrets.py b/tests/test_scan_secrets.py index bf5ca26..ca1322d 100644 --- a/tests/test_scan_secrets.py +++ b/tests/test_scan_secrets.py @@ -36,6 +36,26 @@ def test_real_looking_assignment_flagged(self): found = _scan('password = "h8s7d6f5g4h3j2k1"') self.assertEqual(len(found), 1) + def test_prefixed_secret_names_flagged(self): + # `\b` cannot fire between `_` and a letter, so an earlier version of this + # rule missed every prefixed spelling -- which is most of them in practice. + for line in ('db_password = "h8s7d6f5g4h3j2k1"', + 'my_api_key = "AbCd1234EfGh5678"', + 'STRIPE_SECRET = "AbCd1234EfGh5678"', + 'aws_secret_access_key = "wJalrXUtnFEMI7K7MDENGbPxRfiCYEX"', + "DATABASE_PASSWORD=s3cr3tp4ssw0rd"): + with self.subTest(line=line): + self.assertEqual(len(_scan(line)), 1) + + def test_prefixed_placeholder_still_not_flagged(self): + # Widening the name must not cost the placeholder filter its job. + self.assertEqual(_scan("db_password = changeme"), []) + self.assertEqual(_scan('my_api_key = "your_key_here"'), []) + + def test_short_or_nonsecret_values_not_flagged(self): + self.assertEqual(_scan("password: x"), []) + self.assertEqual(_scan("secret_name = db"), []) + def test_inline_allow(self): fake = "AKIA" + "C" * 16 self.assertEqual(_scan(f"{fake} lore:allow-secret"), []) From 0efe0b56a0f4a3369002878555a73db3569264da Mon Sep 17 00:00:00 2001 From: Angel Date: Sat, 12 Sep 2026 17:12:23 +0200 Subject: [PATCH 2/2] fix: use a personal store here instead of gitignoring the team store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit ignored `learnings/` outright. That got this repo's notes out of the published tree, but by overriding the default the plugin gives everyone else — the team store is committed and shared by default, and that default should stay visible in the plugin's own repo. Use the feature built for this instead: `personalStoreDir` in `.lore.json`, a private second store recall reads alongside the team one (entries surface flagged `[personal]`). `learnings/` goes back to un-ignored; `.lore/` is ignored, and the committed `.lore.json` makes the convention explicit for anyone else working here. Drop the local guards along with it: the pre-push hook, the `.lore/` script copies, and `.github/workflows/lore.yml`. They exist to stop a secret reaching a *published* store, and this repo no longer has one — verified both scripts exit 0 with a message when the store is absent, so the workflow would have been green without checking anything. The script copies were also a duplicate of `plugin/scripts/` two directories away, silently drifting whenever a fix landed in one and not the other, which had already happened once in this branch. Also drop the scaffolded `learnings/` tree: its README and template were copies of `plugin/templates/`, which is in this same repo. Co-Authored-By: Claude Opus 5 --- .gitignore | 10 ++++++---- .lore.json | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 .lore.json diff --git a/.gitignore b/.gitignore index 880afb9..d13a666 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,9 @@ __pycache__/ .DS_Store Thumbs.db -# This repo's own learnings are local dev notes, not something shipped to users -# of the plugin. Un-ignoring them turns them into published content — see the -# security note in learnings/README.md before doing so. -learnings/ +# Personal lore store (`personalStoreDir` in .lore.json) — per-person notes about +# developing this plugin, read by recall alongside the team store but never +# published. `learnings/` is deliberately NOT ignored here: the team store is +# committed and shared by default, and that default is what users of the plugin +# get. +.lore/ diff --git a/.lore.json b/.lore.json new file mode 100644 index 0000000..b8ce356 --- /dev/null +++ b/.lore.json @@ -0,0 +1,3 @@ +{ + "personalStoreDir": ".lore/personal" +}