Stop the secret check from blocking on Django's self.client.session (#4209) (#4228)

* Add failing tests for env.kv flagging a Python attribute read like self.client.session (#4209)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Read a Python or Dart attribute reference assigned with = as a name in env.kv (#4209)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/fix-env-kv-django-session-4209.md

@@ -0,0 +1,4 @@
+---
+important: blocker
+---
+- **The pregate secrets check no longer stops /ship on Django's `session = self.client.session`.** env.kv read any assignment to a name ending in `session`, `token` or `key` as a credential when the value looked random enough, so a Python test that grabbed the test client's session failed a check nobody can downgrade, and the only way past it was a hand-edited allowlist (#4209). In Python and Dart source, an unquoted dotted name such as `self.client.session` or `request.auth`, alone on the right of `=`, now reads as an attribute. A quoted value, a random-looking dotted token, the same line inside a `"""` string, anything after it on the line, and every .env, YAML, config and TypeScript file still report. Reported by @dgrant in #4209.

lib/redact-patterns.ts

@@ -832,8 +832,9 @@ function isCallExpression(span: string, match: RegExpExecArray): boolean {
  * `API_KEY=VelvetRiverOrbitSunset;` in a .env or YAML file is still a literal.
  * #3630 (#3418, #3419): Dart and Python (.py, .pyi) qualify on the same grammar
  * fact, no unquoted string literal, so `'password': request.password` and
- * `session: HttpSession,` are names. Python's `=` forms
- * (`password=request.password`) still fire: the separator must be `:`.
+ * `session: HttpSession,` are names. Python's keyword form
+ * (`password=request.password,`) still fires: the separator must be `:`. A
+ * plain `=` assignment of a dotted name is isAttributeReference's (#4209).
  * Without a known source path nothing is exempt.
  */
 const CODE_SOURCE = /\.(?:[cm]?tsx?|jsx|dart|pyi?)$/i;
@@ -1086,6 +1087,30 @@ function isSourceExpression(span: string, match: RegExpExecArray, sourcePath: st
   return !insideString(match.input, start, sourcePath);
 }
 
+/**
+ * #4209: in Python and Dart source, an unquoted dotted name chain that is the
+ * whole right-hand side of an `=` assignment (`session = self.client.session`,
+ * `token = request.auth`) reads an attribute; it holds no credential. Only
+ * Python and Dart, where the whole file's strings are known (insideString), so
+ * the same line inside a `"""`/`'''` block still reports. Every segment must be
+ * name-shaped (snake_case, camelCase, PascalCase or CONSTANT_CASE), so a dotted
+ * random token still reports, and only a `;` and a comment may follow it on
+ * the line: a literal after it (`or "…"`, `, "…"`), a continuation or a
+ * keyword argument's comma still reports. A quoted value, a bare name, a
+ * config, .env or TS file, and a scan without a path are unchanged.
+ */
+const ATTRIBUTE_SEGMENT = /^_*(?:[a-z][a-z0-9]*(?:_+[a-z0-9]+)*|[A-Z][A-Z0-9]*(?:_+[A-Z0-9]+)*|[A-Za-z][a-z0-9]*(?:[A-Z][a-z0-9]+)*)_*$/;
+function isAttributeReference(span: string, match: RegExpExecArray, sourcePath: string | undefined): boolean {
+  if (!sourcePath || !STRING_SPAN_SOURCE.test(sourcePath)) return false;
+  const chain = span.replace(/;$/, "").split(".");
+  if (chain.length < 2 || !chain.every((segment) => ATTRIBUTE_SEGMENT.test(segment))) return false;
+  const { start, end } = spanBounds(match);
+  if (!/=[ \t]*$/.test(match.input.slice(match.index, start))) return false;
+  const rest = match.input.slice(end).split("\n", 1)[0];
+  if (!(/\.dart$/i.test(sourcePath) ? /^[ \t]*(?:\/\/.*)?\r?$/ : /^[ \t]*(?:#.*)?\r?$/).test(rest)) return false;
+  return !insideString(match.input, start, sourcePath);
+}
+
 /** The files isPropertyRead exempts in: TS, Dart and Python. JSX text in .tsx and .jsx can hold a host on any line, so those report as before. */
 const PROPERTY_READ_SOURCE = /\.(?:[cm]?ts|dart|pyi?)$/i;
 /** How far back on its line isPropertyRead reads from a span. A span further from its line start than this is never exempted. */
@@ -1549,6 +1574,7 @@ export const PATTERNS: RedactPattern[] = [
     // itself (`process.env.X||"…"`) is not an exact read and still fires.
     // #2899: a function call assigned to the name is code, not a value (see
     // isCallExpression). #3048, #3630: so is a TS/JSX/Dart/Python type or expression (isSourceExpression).
+    // #4209: so is a dotted attribute read assigned with = in Python or Dart (isAttributeReference).
     // #3208: so is a schema field identifier (isSchemaFieldIdentifier).
     // GST-1894: so is a self-named fixture label in a test file (isTestFixtureLabel).
     validate: (span, match, opts) =>
@@ -1558,6 +1584,7 @@ export const PATTERNS: RedactPattern[] = [
       !isBareEnvRead(span, match) &&
       !isCallExpression(span, match) &&
       !isSourceExpression(span, match, opts?.sourcePath) &&
+      !isAttributeReference(span, match, opts?.sourcePath) &&
       !isSchemaFieldIdentifier(span, match) &&
       !isTestFixtureLabel(span, opts?.sourcePath) &&
       shannonEntropy(span) >= 3.0,

test/pregate-secrets-source-context.test.ts

@@ -88,6 +88,22 @@ describe('pregate secrets: source context and diff integrity', () => {
     expect(repo({ 'notes.md': `logs are under /home/garry/work/run.log\nkey ${AWS}\n` }).secrets().status).toBe('fail');
   });
 
+  // Value: protects=a Django test that adds `session = self.client.session` passes the never-downgradable secrets check while the same line in a string or an env file still fails it;
+  //   fails_when=the #4209 false positive returns, or the exemption reaches a """ block or a .env value; why_new=#4209 stopped /ship for Python users; seam=fixture repo; no_claim=Dart files at the pregate level
+  test('a Django test client session read passes; the same line in a string or a .env file still fails (#4209)', () => {
+    const base = 'from django.test import TestCase\n\n\nclass ViewTests(TestCase):\n    def test_index(self):\n        self.client.get("/")\n';
+    const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'tests/test_views.py': base } });
+    try {
+      const secrets = () => JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets');
+      const added = base + '        session = self.client.session\n        session["cart"] = [1]\n        session.save()\n';
+      commit(fx, { 'tests/test_views.py': added });
+      expect(secrets().status).toBe('pass');
+      commit(fx, { 'tests/test_views.py': added + '\nFIXTURE = """\n        session = self.client.session\n"""\n', '.env': 'SESSION=self.client.session\n' });
+      const check = secrets();
+      expect({ status: check.status, rows: (check.lines as string[]).filter((l) => l.includes('env.kv')) }).toEqual({ status: 'fail', rows: [expect.stringContaining('.env, added line 1)'), expect.stringContaining('test_views.py, line 12)')] });
+    } finally { cleanup(fx); }
+  });
+
   test('parseAddedLineNumbers numbers each added line from its hunk header', () => {
     const diff = ['diff --git a/a.py b/a.py', '--- a/a.py', '+++ b/a.py', '@@ -3,0 +4,2 @@', '+one', '+two', '@@ -9 +11 @@', '-old', '+new'].join('\n');
     expect([...parseAddedLineNumbers(diff)]).toEqual([['a.py', [4, 5, 11]]]);

test/redact-engine.test.ts

@@ -314,6 +314,8 @@ describe("MEDIUM demoted credential-shaped patterns (TENSION-1)", () => {
       ["    login('it\\'s', password: request.password);", "lib/a.dart"], ['    "user": "a", "password": request.password,', "app.py"],
       // The triple-quote guard is Dart/Python only: a stray ''' in a TS string leaves #3048 as it was.
       ["const s = \"'''\";\n  session: SessionState,", "src/a.ts"],
+      // #4209: a plain assignment of a dotted name reads an attribute; the keyword form below still fires.
+      ["      password = request.password", "app/api/handlers.py"],
     ] as const) expect({ code, file, kv: kv(code, file) }).toEqual({ code, file, kv: false });
     for (const [literal, file] of [
       ["    session: HttpSession,", "deploy/values.yaml"],
@@ -322,7 +324,6 @@ describe("MEDIUM demoted credential-shaped patterns (TENSION-1)", () => {
       [`      "password": "${v}",`, "app/api/handlers.py"],
       [`  password = "${v}"`, "app/config.py"],
       ["      password=request.password,", "app/api/handlers.py"],
-      ["      password = request.password", "app/api/handlers.py"],
       [`CONFIG = """\npassword: ${vIdent}\n"""`, "tests/conftest.py"],
       [`      'password': '${v}',`, "backend/lib/src/handlers/onboarding_handlers.dart"],
       [`  password: "${v}",`, "backend/lib/src/config.dart"],

test/redact-env-kv-attribute-reference.test.ts

@@ -0,0 +1,84 @@
+/**
+ * #4209: env.kv flagged the Django test-client idiom `session = self.client.session`
+ * in a Python test file, and the pregate secrets stage (never downgradable)
+ * stopped /ship on it. An unquoted dotted attribute chain that is the whole
+ * right-hand side of an `=` assignment in Python or Dart source reads a
+ * name; it holds no credential. The hold cases below are the same shapes
+ * carrying a real value, or the same line where it is text, and must still fire.
+ */
+// Value: protects=env.kv stays fail-closed on literals, tokens, env files and strings while a source attribute read passes; fails_when=the attribute-reference exemption reaches a quoted, token-shaped, string or config value; why_new=#4209 had no test of an = assignment reading an attribute; seam=none; no_claim=scanner-level only; the pregate end-to-end case lives in pregate-secrets-source-context.test.ts
+import { describe, expect, test } from "bun:test";
+import { scan } from "../lib/redact-engine";
+
+const kv = (text: string, sourcePath?: string) =>
+  scan(text, { repoVisibility: "private", processEnv: {}, ...(sourcePath ? { sourcePath } : {}) }).findings.some((f) => f.id === "env.kv");
+const v = "8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ";
+const tokenShaped = ["NzE4MjkzNjQ1MDcyODM5Mjk0", "Gx7QpL", "Kq9wErTy5UiOp3AsDf7GhJkL2Zx"].join(".");
+const django = "        session = self.client.session";
+
+describe("env.kv: an attribute reference assigned in source is a name (#4209)", () => {
+  test("hold: quoted literal values still fire", () => {
+    for (const [literal, file] of [
+      [`        session = "${v}"`, "tests/test_views.py"],
+      [`        session = '${v}'`, "tests/test_views.py"],
+      ['        session = "self.client.session"', "tests/test_views.py"],
+      ["        session = 'self.client.session'", "tests/test_views.py"],
+      ['    session = "widget.session.value";', "lib/a.dart"],
+    ] as const) expect({ literal, file, kv: kv(literal, file) }).toEqual({ literal, file, kv: true });
+  });
+
+  test("hold: high-entropy tokens still fire, unquoted ones included", () => {
+    for (const [literal, file] of [
+      [`    token = ${v}`, "app/views.py"],
+      [`    token = ${tokenShaped}`, "app/views.py"],
+      [`    session = ${tokenShaped};`, "lib/a.dart"],
+      ["    token = 8Fk2pQ9v.Xz4wL7mN3rT6", "app/views.py"],
+      ["    token = Fk2p-Q9vX.z4wL7mN3rT6", "app/views.py"],
+      ["    token = abc.def/ghi+jkl==", "app/views.py"],
+    ] as const) expect({ literal, file, kv: kv(literal, file) }).toEqual({ literal, file, kv: true });
+  });
+
+  test("hold: env-style assignments and config files still fire, a dotted value included", () => {
+    for (const [literal, file] of [
+      [`SESSION_TOKEN=${v}`, ".env"],
+      ["SESSION=self.client.session", ".env"],
+      ["export SESSION=self.client.session", "deploy/env.sh"],
+      ["API_KEY=app.settings.api_key", ".env.production"],
+      ["session = self.client.session", "config/app.ini"],
+      ["session = self.client.session", "pyproject.toml"],
+      ["session: self.client.session", "deploy/values.yaml"],
+      ["  session = this.client.session;", "src/a.ts"],
+      [django, undefined],
+      [django, "added.txt"],
+    ] as const) expect({ literal, file, kv: kv(literal, file) }).toEqual({ literal, file, kv: true });
+  });
+
+  test("hold: the same line inside a string or followed by more value still fires", () => {
+    for (const [literal, file] of [
+      [`CONFIG = """\n${django}\n"""`, "tests/conftest.py"],
+      [`CONFIG = f'''\n${django}\n'''`, "tests/conftest.py"],
+      [`CFG = "db \\\n${django} \\\n"`, "app.py"],
+      [`const fixture = '''\n  session = widget.client.session;\n''';`, "test/fixtures.dart"],
+      [`${django} or "${v}"`, "tests/test_views.py"],
+      [`${django}, "${v}"`, "tests/test_views.py"],
+      [`${django} + "${v}"`, "tests/test_views.py"],
+      [`${django} \\\n    + "${v}"`, "tests/test_views.py"],
+      ["      password=request.password,", "app/api/handlers.py"],
+    ] as const) expect({ literal, file, kv: kv(literal, file) }).toEqual({ literal, file, kv: true });
+  });
+
+  test("the Django test-client line and other attribute reads in Python and Dart source pass", () => {
+    for (const [code, file] of [
+      [django, "tests/test_views.py"],
+      [django, "app/tests/test_views.py"],
+      [`class T(TestCase):\n    def test_x(self):\n${django}\n        session["k"] = 1\n`, "tests/test_views.py"],
+      [`${django}  # Django test client`, "tests/test_views.py"],
+      [`${django}\r`, "tests/test_views.py"],
+      ["    token = request.auth", "app/views.py"],
+      ["    self.session = self.client.session", "tests/test_views.py"],
+      ["      password = request.password", "app/api/handlers.py"],
+      ["    session = widget.session;", "lib/a.dart"],
+      ["    session = widget.client.session; // from the parent", "lib/a.dart"],
+    ] as const) expect({ code, file, kv: kv(code, file) }).toEqual({ code, file, kv: false });
+  });
+});