Repository navigation
Scan the pull request description for home paths and credentials - #991
seathatflowsinourveins wants to merge 4 commits into
Conversation
seathatflowsinourveins
added
the
lane:foundation
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de4eed8ffc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const forms = [ | ||
| body, | ||
| body.replace(/(?:%[0-9A-Fa-f]{2})+/g, percentRun), | ||
| body.replace(/\\u([0-9A-Fa-f]{4})|\\\//g, (_, hex) => (hex ? String.fromCharCode(parseInt(hex, 16)) : '/')), | ||
| body.replace(/&(?:#([0-9]{1,7})|#[xX]([0-9A-Fa-f]{1,6})|([A-Za-z]{2,8}));/g, |
There was a problem hiding this comment.
When a private value uses more than one rendering feature—for example, /home/**alice**/x—each entry in forms starts independently from the raw body, so entity decoding leaves the emphasis markers while Markdown cleanup leaves the entity references. GitHub nevertheless displays /home/alice/x, and the current gate passes it without a privacy finding. Parse/render the body once (preferably with a pinned GitHub-compatible implementation) or compose these transformations, and add combined-feature regression cases; the handwritten rendering mechanism currently has no pinned upstream implementation cited as required for this correction.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| const MAX_BODY = 65536; | ||
| const raw = pullRequest.body ?? ''; | ||
| const refused = typeof raw !== 'string' ? 'body-type' | ||
| : raw.length > MAX_BODY ? 'body-size' |
There was a problem hiding this comment.
This merge-blocking guard assumes that GitHub's reported 65,536-character limit uses JavaScript UTF-16 code units, but raw.length counts an astral character twice. The decision record explicitly says the REST documentation provides no limit and that no body of this size was submitted, while the only supporting refusal text comes from an unrelated repository's PR rather than a pinned GitHub source. Verify the boundary and counting unit against the actual API (or remove the speculative guard) before enforcing it; synthetic tests merely reproduce this assumption and do not satisfy the repository's upstream-source requirement for a correction.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| .replace(inlineTags, '').replace(/\\([!-\/:-@\[-`{-~])/g, '$1').replace(/`/g, '') | ||
| .replace(/(\*{1,3}|~{1,2}|_{1,3})([\p{L}\p{N}][^\s*~_\/\\]*?)\1/gu, '$2'), | ||
| ]; | ||
| const hits = privacy.filter(([, pattern]) => forms.some((form) => pattern.test(form))).map(([id]) => id); |
There was a problem hiding this comment.
For a public repository, this workflow runs only after an opened or edited PR description has already been stored and made public, so by the time pattern.test(form) reports a credential or home path, the claimed privacy exposure has already happened. The job has only pull-requests: read, and failing a required check neither removes the body nor prevents it from being viewed or harvested. If this correction is intended to prevent disclosure rather than merely detect it after publication, validation must occur in the PR creation/editing path before submission; this action can remain only a secondary alert.
Useful? React with 👍 / 👎.
| const privacy = [ | ||
| ['personal-home-path', rule(String.raw`\/(?:home|Users)\/(?!example(?:\/|${b}))[A-Za-z0-9_.\-]+(?:\/|${b})`)], | ||
| ['windows-user-path', rule(String.raw`(?:[${AZ}]:[\/\\]+|\/[Mm][Nn][Tt]\/[${AZ}]\/)${users}[\/\\]+(?!${example}(?:[\/\\]|${b}))[${AZ}0-9_.\-]+`)], | ||
| ['encoded-home-path', rule(String.raw`(?<![${W}\-])(?:-home-|-${users}-|[${AZ}]--${users}-|-[Mm][Nn][Tt]-[${AZ}]-${users}-)(?!example(?:-|[\/\\"'${S}\x60)\]]|${end}))[A-Za-z0-9_.]+(?:-|(?=[\/\\"'${S}\x60)\]]|${end}))|(?<![\p{L}\p{Nd}\p{Nl}\p{No}])home-(?!example-code-)[${W}.]+(?:-+[${W}.]+)*-*-code-`)], |
There was a problem hiding this comment.
The second encoded-home alternation classifies any home-…-code-… identifier after punctuation as a private Claude project path, even when it is an ordinary source name. For example, a required SOTA entry containing https://github.com/acme/home-assistant-code-review fails with encoded-home-path, preventing the PR from satisfying the repository's source-description requirement despite containing no home directory. Require the surrounding encoded directory structure or another path-specific prefix instead of applying this fragment to arbitrary repository names and URLs.
AGENTS.md reference: AGENTS.md:L12-L12
Useful? React with 👍 / 👎.
| ['bearer-credential', rule(String.raw`${b}[Bb][Ee][Aa][Rr][Ee][Rr][${S}]+[${AZ}0-9._~\-]{30,}`)], | ||
| // Only here: a macOS home in any case at the start of a path (macOS volumes are case-insensitive by default). | ||
| // validate.py keeps the exact /Users/ because API routes in tracked evidence spell /users/ the same way. | ||
| ['macos-user-path', rule(String.raw`(?<![${W}.~\/\-])\/(?!Users\/)[Uu][Ss][Ee][Rr][Ss]\/(?!example(?:\/|${b}))[A-Za-z0-9_.\-]+`)], |
There was a problem hiding this comment.
A lowercase or mixed-case macOS home such as ///users/alice/x passes the gate because the negative lookbehind rejects the /users match when its slash is preceded by another slash. On POSIX systems, more than two leading slashes resolve as a single root separator, so this still identifies the same home path that /users/alice/x detects. Normalize redundant leading separators or allow this root-path case without broadening matches to ordinary URL path components.
Useful? React with 👍 / 👎.
| ['github-token', rule(String.raw`${b}(?:gh[pousr]_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{30,})${b}`)], | ||
| ['tavily-token', rule(String.raw`${b}tvly-(?:(?:prod|dev)-)?[A-Za-z0-9_\-]{24,}${b}`)], | ||
| ['api-secret', rule(String.raw`${b}sk-(?:proj-|ant-)?[A-Za-z0-9_\-]{24,}${b}`)], | ||
| ['private-key', rule(String.raw`-----BEGIN (?:RSA |EC |OPENSSH |DSA )?PRIVATE KEY-----`)], |
There was a problem hiding this comment.
A PKCS#8 encrypted key begins with -----BEGIN ENCRYPTED PRIVATE KEY-----, but this alternation accepts only an absent prefix or RSA, EC, OPENSSH, or DSA, so a PR body containing that real private-key header passes the gate. This leaves a common sensitive-key representation completely outside a change intended to catch pasted credentials; derive the recognized labels from a pinned upstream key-format source and add a regression fixture for an encrypted PEM key.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| python = {"personal-home-path": "personal home path", "windows-user-path": "Windows user path", | ||
| "encoded-home-path": "encoded home path", "hugging-face-token": "Hugging Face token", | ||
| "github-token": "GitHub token", "tavily-token": "Tavily token", "api-secret": "API secret", | ||
| "private-key": "private key", "bearer-credential": "bearer credential"} |
There was a problem hiding this comment.
The parity test iterates only this hand-maintained python mapping and never asserts that it covers every entry in validate.PRIVATE_CONTENT other than the two intentional session exclusions. If validate.py later adds or renames a credential rule, all of these tests can still pass while the PR gate silently stops matching the repository's publication scanner. Assert equality between the source rule names and the mapped names (with an explicit exclusion set), as well as coverage by the JavaScript rule IDs.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d81969ddcd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
| const links = /!?\[([^\]\n]*)\]\(\s*<?([^\s()<>]*)>?(?:\s+(?:"[^"\n]*"|'[^'\n]*'|\([^()\n]*\)))?\s*\)/g; | ||
| // Only the inline elements GitHub renders are removed; any other <word>, such as a <user> placeholder, stays as text. | ||
| const inlineTags = /<\/?(?:a|abbr|b|bdi|bdo|br|cite|code|del|dfn|em|i|img|ins|kbd|mark|q|rp|rt|ruby|s|samp|small|span|strike|strong|sub|sup|time|tt|u|var|wbr)\b[^<>\n]*>/gi; |
There was a problem hiding this comment.
When a private value is split inside its fixed prefix with an HTML comment, such as /ho<!--x-->me/alice/x or gh<!--x-->p_..., GitHub renders the comment invisibly and displays the continuous sensitive value, but inlineTags does not remove comments, so none of the composed forms match. Fresh evidence in this revision is that both workflow copies still pass these inputs after the decoder-composition fix; add GFM-compatible comment handling and a regression case.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| // reads: percent escapes, JSON escapes, HTML character references, and Markdown's rendered text (the body itself | ||
| // keeps link destinations). Emphasis goes only in pairs around a word without a separator in it, so a glob such as | ||
| // Users/*/ keeps its star. | ||
| const fromCode = (code) => (code <= 0x10FFFF && !(code >= 0xD800 && code <= 0xDFFF) ? String.fromCodePoint(code) : ''); |
There was a problem hiding this comment.
When otherwise benign PR prose contains an invalid numeric character reference inside a path-like string, such as /ho�me/alice/x, GitHub's Markdown parsing renders a replacement character, but fromCode returns an empty string and joins the fragments into /home/alice/x. This makes the required check reject text that never renders a private path; preserve the reference or emit the parser's replacement character and cover the failing case.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
d81969d to
fc60da9
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc60da90a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Only here: the graph-id form (home-<name>-code-, no leading dash) is not read within 256 characters after an | ||
| // http(s) scheme, where it names a public repository or page, such as github.com/<owner>/home-<name>-code-review. | ||
| // The check runs only where home- occurs, so it stays linear. | ||
| ['encoded-home-path', rule(String.raw`(?<![${W}\-])(?:-home-|-${users}-|[${AZ}]--${users}-|-[Mm][Nn][Tt]-[${AZ}]-${users}-)(?!example(?:-|[\/\\"'${S}\x60)\]]|${end}))[A-Za-z0-9_.]+(?:-|(?=[\/\\"'${S}\x60)\]]|${end}))|(?<![\p{L}\p{Nd}\p{Nl}\p{No}])home-(?<!https?:\/\/[^${S}<>"'\x60]{0,256}home-)(?!example-code-)[${W}.][${W}.\-]{0,255}-code-`)], |
There was a problem hiding this comment.
When a graph-encoded local path appears in an HTTP URL query or fragment, this negative lookbehind exempts it solely because https:// occurs within 256 characters; for example, https://example.org/?file=~/.cache/codebase-memory-mcp/home-alice-code-secret.db passes although the identical cache identifier outside the URL is caught. Fresh evidence in this revision is that the repository-name exemption added after the prior review also suppresses these genuinely private path values. Restrict the exemption to repository-shaped URL components and add a regression case for a URL carrying a local path.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| (text) => text.replace(/\\u([0-9A-Fa-f]{4})|\\\//g, (_, hex) => (hex ? String.fromCharCode(parseInt(hex, 16)) : '/')), | ||
| (text) => text.replace(/&(?:#([0-9]{1,7})|#[xX]([0-9A-Fa-f]{1,6})|([A-Za-z]{2,8}));/g, | ||
| (whole, decimal, hex, name) => (decimal ? fromCode(Number(decimal)) : hex ? fromCode(parseInt(hex, 16)) : named.get(name) ?? whole)), | ||
| (text) => text.replace(links, '$1').replace(/!?\[([^\]\n]*)\]\[[^\]\n]*\]/g, '$1').replace(/<((?:https?|file|mailto):[^<>\s]*)>/g, '$1') |
There was a problem hiding this comment.
When text resembles a reference link but has no matching definition, such as /home/[al][missing]ice/x, GFM leaves the brackets and label as literal text, but this unconditional replacement collapses it to /home/alice/x. The merge-blocking check therefore rejects benign prose for a private path that is neither stored nor rendered contiguously. Parse reference definitions before collapsing labels, or use a pinned GFM-compatible parser, and add an unresolved-reference regression case.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
The command center's correction #145 found the host account name, inside absolute home paths, in eleven public PR bodies and comments. The required sota-sources job (both byte-identical copies) now also checks the current PR body against scripts/validate.py's home-path and credential rules, written in JavaScript, and fails closed naming only the rule ids. ~/, <user> and the example home pass. A test runs every rule against its Python original on a shared corpus; the new tests fail on the job before this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efuse odd bodies The co-op's GPT read at 0759532 found three gaps, closed in both copies of the sota-sources job: - The body is refused before it is read when it is not text, is longer than 65,536 characters (the API's own "maximum is 65536 characters"), holds a lone surrogate, or holds a control character other than tab, LF or CR; the failure names one fixed rule id. - The rules also run over the body's percent, JSON and HTML decodings and its rendered Markdown text, each decoded once. The rendered form drops only GitHub's inline HTML elements and paired emphasis, so a <name> placeholder or a Users/*/ glob stays as written. A gate-only macos-user-path rule catches /users/<name> in any other case. - The rules spell out Python's str-pattern classes (Unicode \w, \s, boundaries, $, and re.IGNORECASE's extra letters) under the u flag, without the i flag. A new test compares each rule with scripts/validate.py over 156,277 code points in 69 templates. Every open pull request's live description gets the same outcome as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex's GitHub review of de4eed8 (4 P1, 3 P2), in both copies of the job: - Correction: GitHub stores descriptions over 65,536 characters (its REST update keeps up to 262,144 UTF-8 bytes; #786 has 75,726). The bound is the gate's own, 262,144 code points. - The four decoders compose: every order of every subset, each at most once along a chain (65 forms). - validate.py and the gate take gitleaks's private-key header form, so PKCS #8 ENCRYPTED and OpenPGP key blocks are caught. - The gate does not read the graph-id form inside an http(s) URL; the graph-id name runs at most 256 characters to -code- in both scans (a quadratic 21-93 s run is now 0.05-0.1 s). - macos-user-path takes any number of leading slashes. - A test fails when validate.py gains a rule the gate does not carry. - Publication order is answered in the record: the gate is the backstop; prevention is scanning before gh pr create or edit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…bels The co-op's GPT read of fc60da9 (2 P2), in both copies of the job: - The graph-id URL exemption covered any http(s) URL, so a graph id behind a loopback or localhost URL passed. It now covers only the path of a public code host's URL (an allowlist, lower case, up to four subdomain labels, stopping at : @ ? #), so loopback, localhost, private-address and unlisted hosts stay caught. - Link and reference labels admitted '[', so 262,144 unmatched '[' gave no verdict within 125 s. Labels now hold no bracket (CommonMark); that body takes 0.16 s, and 21 adversarial shapes at the bound at most 0.26 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fc60da9 to
4e5ef13
Compare
This branch has not been deployed
1 participant
Scope
sota-sourcesjob also scans the pull request description for an absolute home path or a credential-shaped string, and fails closed naming only the rule id. This prevents a repeat of the command center's privacy correction Blind lanes round 2: sanitized export, receipt aliases, two-family adjudication #145: home paths had reached eleven places in public PR bodies and comments, all masked since.0f1537841(rebased fromc4f499073for a manifest-only conflict; the earlier commits' change outsidemanifests/evidence.jsonis byte-identical).lane:foundation..github/workflows/pr-metadata.ymland.github/workflows/sota-sources-gate.yml(the same job, byte for byte),scripts/validate.py(the private-key and graph-id rules),tests/test_sota_sources_gate.py,docs/decisions/2026-09-25-top-rule-sota-sources.md,manifests/evidence.json.How:
scripts/validate.py'sPRIVATE_CONTENT, written in JavaScript in the job's github-script: personal home path, Windows user path, encoded home path, and the Hugging Face, GitHub, Tavily, API-secret, private-key and bearer-credential rules, plus a gate-only macOS home rule in any case.~/paths,<user>and the documentedexamplehome pass.Review rounds:
07595323(three P2s) was fixed inde4eed8f: body guards, decoded forms, Python's Unicode classes spelled out.de4eed8f(four P1s, three P2s) is answered infc60da90:d81969ddhad claimed a 1,048,577-character body was stored, from that 200 answer; a GET read-back corrected it.)ENCRYPTEDand OpenPGP key blocks.validate.pygains a rule the gate does not carry.gh pr createorgh pr edit.fc60da90(two P2s) is answered in4e5ef13d:: @ ? #. Loopback, localhost, private-address and unlisted hosts stay caught.[, so 262,144 unmatched[gave no verdict within 125 s. Labels now hold no bracket, as in CommonMark. That body takes 0.16 s, and 21 adversarial shapes at the bound take at most 0.26 s each.SOTA sources
body, which the job already reads throughgithub.rest.pulls.getinactions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3(v9.0.0).scripts/validate.py,PRIVATE_CONTENTatc4f499073(lines 38-61): the rules ported.tests.test_sota_sources_gatekeeps the JavaScript and Python versions in agreement, over a shared corpus and every assigned code point.config/gitleaks.toml, ruleprivate-key, at83d9cd684c87(https://github.com/gitleaks/gitleaks/blob/83d9cd684c87d95d656c1458ef04895a7f1cbd8e/config/gitleaks.toml#L2784-L2788): the private-key header form, kept case-sensitive.Evidence-class table
native_provengh api -X PATCHprobes on this PR, checked by GET read-back and the edit history, each restored and read back equal; receipt kept by the api-actions lane~/,<user>andexamplepasssyntheticGatePrivacyTests, which runs the job's script in node with the github-script calling conventionsynthetictest_each_rule_agrees_with_scripts_validate,test_each_rule_agrees_with_python_on_every_code_pointsynthetictest_a_body_at_the_bound_is_scanned_in_bounded_timede4eed8f, and each of 11 weakened copies fails themsyntheticFAILED (failures=22); 11 of 11 mutants faillocal_integrationde4eed8f; 86 at 20:17Z againstfc60da90; rule ids onlysynthetictest_a_body_at_the_bound_is_scanned_in_bounded_time; 21 shapes at 262,144 code points, at most 0.26 s eachLocal commands run
Decision record
docs/decisions/2026-09-25-top-rule-sota-sources.md, "PR-body privacy scan (2026-10-10)", "After the first review", "After Codex's review (2026-10-11)" and "After the second GPT read (2026-10-11)".🤖 Generated with Claude Code