Repository navigation
fix(scan): stop reading a URL as XPath and an interpolated DSN as a credential - #203
Merged
Merged
Conversation
…redential
Two false positives with one cause: a pattern that matches the shape of a
credential without asking whether the file could contain one.
py-xpath-injection matched any f-string holding `//` plus an
interpolation, so every `f"https://{host}/v1/{path}"` in Python was a
high-severity XPath injection. Its JS twin already joins on an xpath
library import (`fileRequires`); the Python rule had no such join. Add
one -- nothing in the standard library evaluates XPath without lxml or
ElementTree -- and exclude the `//` of a URL scheme from the pattern, so
the rule is wrong on neither half alone.
secret-database-url already exempts a DSN whose password slot is a
template expression, but INTERPOLATION knew only `{{...}}`, `${...}`,
`$VAR` and `%s`. It did not know the single brace of a Python f-string,
which is what the code that *builds* a DSN actually writes, nor the
`<pw>` of a usage line -- KNOWN_PLACEHOLDERS has a `<[a-z-]+>` entry but
it is `\b`-anchored, and there is no word boundary between a DSN's `:`
and the `<` after it. Teach INTERPOLATION both.
Measured on the pull request that surfaced this (profullstack/cli-tools
#103, a Python ops CLI): 20 findings -> 5, and all five that remain are
real -- two committed Supabase JWTs, one live Postgres password, and two
interpolated SQL statements. The 15 that went away were eleven URLs read
as XPath, two DSNs assembled from a variable, and one usage comment.
A literal password in the same slot is still reported, as is a templated
username beside a real password: only the password slot decides it.
ThreatCrush Security Scan12 finding(s) HIGH/CRITICAL: 1 | MEDIUM: 6 | LOW: 5
Snippets are redacted; ThreatCrush never prints matched credential material. |
This was referenced Sep 25, 2026
Merged
Contributor
Author
|
Correction to the measurement in the description: the "20 findings → 5" figure was produced by a harness that called The fix itself is fine; CI measured it on the real PR as 16 high + 4 medium → 2 high + 2 medium, with all eleven XPath-on-a-URL alerts gone. The two remaining highs were a third false positive of the same family, fixed in #207. For anyone measuring this way later: resolve the language as the CLI does, |
ralyodio
added a commit
that referenced
this pull request
Sep 25, 2026
Third false positive in the same family as #203: a pattern matching the shape of a dangerous call without asking whether the danger is reachable. py-ssrf-outbound-request fires on `urlopen(req)` for any variable `req`, so a Python tool that talks to exactly two fixed APIs was reported twice at high severity: req = urllib.request.Request("https://backboard.railway.com/graphql/v2", data=body) req = urllib.request.Request(f"https://api.supabase.com/v1/{path}") SSRF is control of the destination, not of the path. Neither of those can be steered to the metadata endpoint or to localhost for any input, and the first has no interpolation in it at all. `constantRequestHostGuard` resolves the variable to its assignments in the file and exonerates only when every one of them names a literal host. An interpolated authority (`f"https://{host}/v1"`), a bare variable (`Request(url)`), or a name with no assignment in sight all leave the finding standing, and a name assigned in two functions is exonerated only if both destinations are fixed. Measured on profullstack/cli-tools#103 with the language resolved the way the CLI resolves it -- by shebang, since the file is extensionless: 4 findings -> 2, and the two that go are exactly the two GitHub reported at dev2/dev2-site:136 and :1132. The two that remain are the real interpolated SQL statements.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two false positives with one cause: a pattern that matches the shape of a credential without asking whether the file could contain one.
py-xpath-injectionMatched any f-string holding
//plus an interpolation, so everyf"https://{host}/v1/{path}"in Python was a high-severity XPath injection. Its JS twin already joins on an xpath library import (fileRequires); the Python rule had no such join.PY_XPATH_FILE— nothing in the standard library evaluates XPath withoutlxmlorElementTree.//of a URL scheme from the pattern via(?<!:).Both, so the rule is not wrong on either half alone.
secret-database-urlAlready exempts a DSN whose password slot is a template expression, but
INTERPOLATIONknew only{{…}},${…},$VARand%s. It did not know:<pw>of a usage line.KNOWN_PLACEHOLDERShas a<[a-z-]+>entry, but it is\b-anchored and there is no word boundary between a DSN's:and the<after it.Measured
On the pull request that surfaced this (profullstack/cli-tools#103, a Python ops CLI): 20 findings → 5, and all five that remain are real — two committed Supabase JWTs, one live Postgres password, two interpolated SQL statements.
The 15 that went away: 11 URLs read as XPath, 2 DSNs assembled from a variable, 1 usage comment, 1 duplicate.
Not loosened
A literal password in the slot is still reported, and a templated username beside a real password stays reported — only the password slot decides it. Both are asserted in the new tests.
357 tests pass.