diff --git a/packages/scan/src/__tests__/code-rules.test.ts b/packages/scan/src/__tests__/code-rules.test.ts index d8c71c1a..f703ddfb 100644 --- a/packages/scan/src/__tests__/code-rules.test.ts +++ b/packages/scan/src/__tests__/code-rules.test.ts @@ -869,12 +869,36 @@ describe('misconfiguration, weak crypto and injection', () => { it('flags an XPath expression with an inner quote around the interpolation', () => { // The `"`-delimited f-string contains a `'`; a class that excluded both // quotes stopped before the interpolation and missed it. - expect(ruleIds('a.py', "expr = f\"//user[name/text()='{username}']\"")).toContain( - 'py-xpath-injection', - ); + const src = [ + 'from lxml import etree', + 'expr = f"//user[name/text()=\'{username}\']"', + ].join('\n'); + expect(ruleIds('a.py', src)).toContain('py-xpath-injection'); expect(ruleIds('a.py', 'result = doc.xpath("//user[name=$n]", n=username)')).toEqual([]); }); + it('does not read the // of a URL scheme as an XPath step', () => { + // The commonest false positive by far: an f-string holding a URL. `//` + // after a scheme is not a descendant-or-self step, and a file that does no + // XML at all cannot be evaluating XPath. Both halves are asserted here + // because either one alone still let the other shape through. + const urls = [ + 'url = f"https://{host}/v1/{path}"', + 'dsn = f"postgres://postgres:{pw}@{host}:5432/{db}"', + "req = f'redis://{host}:{port}/0'", + ].join('\n'); + expect(ruleIds('a.py', urls)).not.toContain('py-xpath-injection'); + + // Even with lxml in the file, a URL is still a URL. + expect(ruleIds('a.py', ['from lxml import etree', 'url = f"https://{host}/v1"'].join('\n'))).toEqual( + [], + ); + + // And a real XPath in a file that never imports an XML library is out of + // scope, the same way the Java XXE rule joins on its parser imports. + expect(ruleIds('a.py', 'expr = f"//user[@id={uid}]"')).toEqual([]); + }); + it('flags a NoSQL query built from request data', () => { expect(ruleIds('a.js', 'return users.find(req.body.filter);')).toContain('js-nosql-injection'); expect(ruleIds('a.js', 'return users.find({ _id: sanitize(req.params.id) });')).toEqual([]); diff --git a/packages/scan/src/__tests__/false-positives.test.ts b/packages/scan/src/__tests__/false-positives.test.ts index f95e38df..7f859b4b 100644 --- a/packages/scan/src/__tests__/false-positives.test.ts +++ b/packages/scan/src/__tests__/false-positives.test.ts @@ -159,6 +159,30 @@ describe('fixture credentials in test files', () => { expect(ruleIds('tests/db.test.js', dsn)).toContain('secret-database-url'); }); + it('exempts a DSN whose password slot is a Python f-string or a usage line', () => { + // The code that BUILDS a connection string holds the password in a + // variable; the interpolation is the proof it is not a credential. Both + // shapes came off one pull request -- the f-string from the tool that + // assembles the DSN, the angle brackets from the usage comment above it. + const templated = [ + 'url = f"postgres://postgres:{pw}@{host}:5432/{db}"', + "dsn = f'mysql://{user}:{password}@{host}/{name}'", + '# CLOUD_DB_URL=postgres://postgres.:@aws-0-.pooler.supabase.com:5432/postgres', + ]; + for (const line of templated) { + expect(ruleIds('dev2/dev2-site', line), line).not.toContain('secret-database-url'); + } + + // A literal password in the same slot is still material, and a templated + // username beside a real password stays reported: only the password slot + // decides it. + expect(ruleIds('dev2/sites.d/site.json', 'postgres://postgres:7f3a9c21e4@db.internal:5432/app')) + .toContain('secret-database-url'); + expect(ruleIds('a.py', 'url = f"postgres://{user}:hunter2hunter2@{host}/app"')).toContain( + 'secret-database-url', + ); + }); + it('does not read a fixture stem out of the middle of a word', () => { expect(isTestFixtureValue("const x = 'a'", 'latestbuildsecret')).toBe(false); expect(isTestFixtureValue("const x = 'a'", 'contestwinner2024')).toBe(false); diff --git a/packages/scan/src/code-rules.ts b/packages/scan/src/code-rules.ts index 1497e9cd..5549c51a 100644 --- a/packages/scan/src/code-rules.ts +++ b/packages/scan/src/code-rules.ts @@ -330,6 +330,14 @@ const XXE_GUARD = const XML_PARSING_FILE = /\b(?:javax\.xml|org\.xml\.sax|org\.w3c\.dom|org\.jdom2?|org\.dom4j|XmlPullParser|DocumentBuilderFactory|DocumentBuilder|SAXParserFactory|SAXParser|XMLInputFactory|XMLReaderFactory|XMLReader|SAXBuilder|SAXReader)\b/; +/** + * Python only evaluates XPath through lxml or ElementTree; nothing else in the + * standard library takes an XPath expression. Joining on the file keeps the + * rule off the far more common f-string that simply holds a URL -- see the + * `://` exclusion in the pattern for the second half of that defence. + */ +const PY_XPATH_FILE = /\b(?:lxml|xml\.etree|ElementTree|etree|defusedxml|xpath)\b/i; + /** A sink that executes whatever string reaches it. */ const CODE_SINK = /\bglobalThis\s*\[|\bconstructor\b|\beval\b|\bFunction\b|\brun\s*\(|\bvm\s*\.\s*run/; @@ -1456,9 +1464,12 @@ export const CODE_RULES: readonly CodeRule[] = [ severity: 'high', languages: ['python'], // Per-quote, so an inner `'` in a `"`-delimited f-string (`text()='{x}'`) - // does not truncate the match before the interpolation. + // does not truncate the match before the interpolation. `(?` of a usage line in a comment or README. Anchored to + * the end of the credential segment so it has to fill the slot rather than + * merely appear somewhere near it. + * + * The single brace and the angle brackets are the two that a tool building + * DSNs actually writes — `f"postgres://postgres:{pw}@{host}/{db}"` is the code + * that assembles a connection string, not a credential sitting in a file. Note + * the angle-bracket form cannot be left to the `<[a-z-]+>` entry in + * KNOWN_PLACEHOLDERS below: that one is `\b`-anchored, and there is no word + * boundary between the `:` of the DSN and the `<` that follows it. */ -const INTERPOLATION = String.raw`(?:\{\{[^}]*\}\}|\$\{[^}]*\}|\$[A-Za-z_]\w*|%[sdv])`; +const INTERPOLATION = String.raw`(?:\{\{[^}]*\}\}|\$\{[^}]*\}|\$[A-Za-z_]\w*|%[sdv]|\{[^{}\s'"@/]+\}|<[A-Za-z_][\w.-]*>)`; const KNOWN_PLACEHOLDERS = [ // Deliberately NOT here: AWS's published documentation key/secret pair