From 73a7267aa64d59eec8d1bf55e2ea6b63e9312a63 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 03:19:06 +0000 Subject: [PATCH] diff-scope: fail-safe auth match; resolveBase never re-qualifies a remote-qualified base (GST-1065, GST-1036) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-diff-scope | 16 +++++++--------- changelog.d/capy-bare-base-reads-originbase.md | 3 ++- docs/review-policy.md | 2 +- lib/gate-diff/git.ts | 3 ++- test/diff-scope.test.ts | 3 ++- test/stale-local-base.test.ts | 11 +++++++++++ 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index 8ae89637e..8671aa7fd 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -129,21 +129,19 @@ _session_data() { } # An auth path holds a token, split the way lib/review-risk.ts tokens() splits -# paths (camelCase, then every non-alphanumeric run), that names auth -# (GST-1065): authoriz or authenticat anywhere in it (unauthenticated, -# deauthorize), oauth at its start (oauth2client), or auth at its start or end -# (auth.ts, /auth/, authn, authz, auth0, basicauth, nextauth). A token starting -# with author (author-spec.md, AUTHORS, authority.ts) is not auth, and oauth -# inside a word (coauthors) is not either. +# paths (camelCase, then every non-alphanumeric run), that contains auth +# (GST-1065). It fails safe: every such token raises the class except the +# authorship words below, so author-spec.md, AUTHORS, authority.ts and +# coauthors.ts do not, while auth.ts, /auth/, nextauthmiddleware.ts, +# noauthrequired.ts, unauthenticated.ts and HTTPAuthorization.ts do. _auth_path() { local s="$1" t r=1 while [[ "$s" =~ ^(.*[a-z0-9])([A-Z].*)$ ]]; do s="${BASH_REMATCH[1]} ${BASH_REMATCH[2]}"; done shopt -s nocasematch for t in ${s//[^a-zA-Z0-9]/ }; do case "$t" in - *authoriz*|*authenticat*|oauth*) r=0; break ;; - author*) ;; - auth*|*auth) r=0; break ;; + author|authors|authored|authoring|authorship|authority|authorities|coauthor|coauthors) ;; + *auth*) r=0; break ;; esac done shopt -u nocasematch diff --git a/changelog.d/capy-bare-base-reads-originbase.md b/changelog.d/capy-bare-base-reads-originbase.md index 98757000c..79d474abe 100644 --- a/changelog.d/capy-bare-base-reads-originbase.md +++ b/changelog.d/capy-bare-base-reads-originbase.md @@ -1,2 +1,3 @@ - **A bare `--base main` reads origin/main in every review and ship bin (GST-1036).** `gstack-review-plan` now prints `REVIEW_BASE: ` and names a lagging local base on stderr. `gstack-banned-terms`, `gstack-prose-lint --base`, `gstack-docs-candidate`, `gstack-design-detect.ts scan --changed` and `gstack-pr-shots --base` no longer merge against a stale local main and report lines the branch only inherited. They all use the one resolver gstack-gate-diff already used. -- **`gstack-diff-scope` matches auth as a word (GST-1065).** `author-spec.md`, `AUTHORS` and `authority.ts` no longer raise a docs diff to gate class; `auth.ts`, `/auth/`, `oauth`, `authn`, `authz` and camelCase names like `useAuth.ts` still do. +- **`gstack-diff-scope` stops treating authorship files as auth (GST-1065).** `author-spec.md`, `AUTHORS`, `authority.ts` and `coauthors.ts` no longer raise a docs diff to gate class. Every other name with auth in it still does, including `auth.ts`, `/auth/`, `oauth`, `nextauthmiddleware.ts` and camelCase names like `useAuth.ts`. +- **A remote-qualified `--base origin/main` is never re-qualified.** When the tracking ref is missing it resolves to nothing instead of trying `origin/origin/main`. diff --git a/docs/review-policy.md b/docs/review-policy.md index 8618fa076..1ce6983bb 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -387,7 +387,7 @@ and matching is token-bounded: `costume` never reads as cost. |---|---| | money | a path segment naming cost, spend, budget, ledger, billing, price, fee or invoice; or a changed line with an identifier ending in `_usd` or `Usd`, any `spend_cap` spelling, or a call to a paid provider `lib/pricing.ts` prices | | gate | a hunk the gate scan tagged: a relaxed threshold, a skipped test, a loosened check | -| auth | `SCOPE_AUTH` from `gstack-diff-scope`, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (`recovered-sessions.jsonl`) outside a config location does not set `SCOPE_AUTH`, and `SCOPE_AUTH` reads auth as a word (`auth.ts`, `/auth/`, `oauth`, `useAuth.ts`, `unauthenticated.ts`), never inside `author` or `authority` | +| auth | `SCOPE_AUTH` from `gstack-diff-scope`, on a code path with an auth, session, jwt, permission or role token in its name; a data file named for sessions (`recovered-sessions.jsonl`) outside a config location does not set `SCOPE_AUTH`, and `SCOPE_AUTH` fails safe: any name token containing auth sets it (`auth.ts`, `/auth/`, `oauth`, `useAuth.ts`, `nextauthmiddleware.ts`) except the authorship words `author`, `authors`, `authored`, `authoring`, `authorship`, `authority`, `authorities`, `coauthor` and `coauthors` | | secret | a secrets, credentials, keychain or token path, or a changed line that reads a key-named environment variable (`OPENAI_API_KEY`, `GH_TOKEN`) or a credential store | An input the classifier cannot read (the diff, `gstack-diff-scope`, the gate diff --git a/lib/gate-diff/git.ts b/lib/gate-diff/git.ts index daebe14d4..76bda416a 100644 --- a/lib/gate-diff/git.ts +++ b/lib/gate-diff/git.ts @@ -109,8 +109,9 @@ export function resolveBase(ref: string, cwd: string, head?: string): { ref: str if (!isBranchName(ref)) { const sha = direct(); return sha ? { ref, sha } : null; } const qualified = resolvesToCommit(`refs/remotes/` + ref, cwd); if (qualified) return { ref, sha: qualified }; - const local = resolvesToCommit(`refs/heads/` + ref, cwd); const remotes = remoteNames(cwd); + if (remotes.some((remote) => ref.startsWith(remote + '/'))) return null; + const local = resolvesToCommit(`refs/heads/` + ref, cwd); const localRelated = !!local && related(local); const twin = (remote: string) => { const sha = resolvesToCommit(`refs/remotes/` + remote + '/' + ref, cwd); diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index 82c67f15b..8d1e12650 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -238,7 +238,8 @@ describe('gstack-diff-scope', () => { for (const file of ['src/auth.ts', 'app/auth/login.rb', 'lib/oauth.ts', 'src/oauth2/token.ts', 'lib/authn/check.go', 'src/authz.py', 'src/authService.ts', 'src/useAuth.ts', 'lib/OAuthClient.ts', 'lib/authentication.rb', 'lib/authorization_policy.rb', 'src/Authorize.tsx', 'config/AUTH_KEYS.md', 'pkg/auth-middleware.go', 'lib/auth0.ts', 'src/basicauth.go', 'src/authguard.ts', 'lib/reauth.ts', 'pages/api/nextauth.ts', 'src/authtoken.ts', 'lib/preauth.rb', 'src/HTTPAuth.ts', - 'src/unauthenticated.ts', 'lib/reauthentication.ts', 'src/deauthorize.ts', 'app/oauth2client.py', 'api/HTTPAuthorization.ts']) { + 'src/unauthenticated.ts', 'lib/reauthentication.ts', 'src/deauthorize.ts', 'app/oauth2client.py', 'api/HTTPAuthorization.ts', + 'src/nextauthmiddleware.ts', 'src/reauthn.ts', 'src/noauthrequired.ts']) { test(`${file} is an auth path`, () => { expect(runScope(createRepo([file])).SCOPE_AUTH).toBe('true'); }); diff --git a/test/stale-local-base.test.ts b/test/stale-local-base.test.ts index 5c2c749d6..7cf99e1f1 100644 --- a/test/stale-local-base.test.ts +++ b/test/stale-local-base.test.ts @@ -11,6 +11,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { BANNED_TERMS_SCHEMA } from '../lib/banned-terms'; +import { resolveBase } from '../lib/gate-diff/git'; import { resolveCommit } from '../lib/pr-shots'; const BIN = path.join(import.meta.dir, '..', 'bin'); @@ -101,6 +102,16 @@ describe('a bare --base reads origin/ over a stale local branch (GST-1036) expect(r.err).toContain(`gstack-docs-candidate: ${NOTE}`); }, 120_000); + // Value: protects=a remote-qualified base is never re-qualified: origin/main reads refs/remotes/origin/main or nothing; + // fails_when=resolveBase falls through to refs/remotes/origin/origin/main when the tracking ref is missing; why_new=GST-1036 review (#3852 rule); seam=lib/gate-diff/git.ts resolveBase + test('resolveBase never re-qualifies origin/main when refs/remotes/origin/main is missing', () => { + const { work, sh, origin } = fixture(); + expect(resolveBase('origin/main', work, 'HEAD')?.sha).toBe(origin); + sh(work, 'git update-ref refs/remotes/origin/origin/main HEAD && git update-ref -d refs/remotes/origin/main'); + expect(resolveBase('origin/main', work, 'HEAD')).toBeNull(); + expect(resolveBase('main', work, 'HEAD')?.ref).toBe('main'); + }, 120_000); + // Value: protects=the PR screenshot's before side is origin's base, not a stale local copy; fails_when=resolveCommit reads refs/heads/main first; // why_new=GST-1036 sibling bin; seam=lib/pr-shots.ts resolveCommit test('gstack-pr-shots --base main reads origin/main; a sha resolves as given', () => {