mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 00:03:15 +01:00
fix(review-risk): unread data files and cost_usd reads no longer raise money (GST-961, GST-962)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
b8f1d1326b
commit
5f43fa66a3
7 files changed
+46
-15
No files matched your search
@@ -0,0 +1,8 @@
|
||||
### Fixed
|
||||
|
||||
- **A docs or data PR no longer gets a money reader because a file name says "cost", and reading a `cost_usd` field no longer counts as money code (GST-961, GST-962).** On gstack-evals#138, a results write-up planned as docs at light depth still dispatched a second, targeted money reader, because `wiki/.../cost-gstack-full.tsv` has "cost" in its name. On gstack-evals#139, an analysis module that only sums and prints per-trial `cost_usd` raised the money class. A data file that no code reads now never counts as money, by its name or its rows, and a `_usd` or `Usd` name counts only when it says cap, limit, max, budget, spend, price, fee, bill or payment, or when a `<` or `>` compares it. Spend caps, budgets, billing paths and paid-provider calls still count. Every other class still reads unread data files. Over the last 100 main commits, no commit's depth, class, risk classes or targeted classes changed.
|
||||
|
||||
### Tests
|
||||
|
||||
- `test/review-targeted.test.ts`: #138's file list plans no targeted reader, and the same file read by code still plans money. Neither a `cost_usd` read nor a `cost_usd` report names money. Positive controls: caps, budgets, limits, spend checks, comparisons, `spend_cap`, a billing path, a provider client, and a secret in an unread data file.
|
||||
- `test/review-risk-redos.test.ts`: `usdMoney` stays under 250 ms on 100,000-character lines of `_usd` names, comparisons and member chains.
|
||||
@@ -385,7 +385,7 @@ and matching is token-bounded: `costume` never reads as cost.
|
||||
|
||||
| Class | Signal |
|
||||
|---|---|
|
||||
| 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 |
|
||||
| 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` whose name says cap, limit, max, budget, ceiling, allowance, spend, price, fee, bill or payment, or that a `<` or `>` compares (reading or reporting `cost_usd` is not money, GST-962); any `spend_cap` spelling; or a call to a paid provider `lib/pricing.ts` prices. A data file no code reads (the same consumer scan the depth uses) never counts as money by its name or its rows (GST-961); every other class still reads it |
|
||||
| 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` 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 |
|
||||
|
||||
+1
-1
@@ -245,7 +245,7 @@ export interface ClassResult { class: DiffClass; reasons: string[]; errors: Dept
|
||||
|
||||
const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200);
|
||||
const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p);
|
||||
const unconsumedData = (i: ClassInput, p: string) => Array.isArray(i.consumed) && !i.consumed.includes(p) && (dataCandidate(p, i.root) || (!!i.drawings?.includes(p) && svgCandidate(p, i.root)));
|
||||
export const unconsumedData = (i: Pick<ClassInput, 'consumed' | 'drawings' | 'root'>, p: string) => Array.isArray(i.consumed) && !i.consumed.includes(p) && (dataCandidate(p, i.root) || (!!i.drawings?.includes(p) && svgCandidate(p, i.root)));
|
||||
const pathClass = (i: ClassInput, p: string): PathClass => (i.excluded?.has(p) || (i.stampOnly ?? []).includes(p) || unconsumedData(i, p) ? 'docs' : classifyPath(p, i.root));
|
||||
|
||||
/** The diff's class, raised to gate by any gate signal; `errors` lists every input that fails the depth closed; `runtimeLines` counts code-class lines outside the base registry's outputs. */
|
||||
|
||||
+20
-6
@@ -22,12 +22,23 @@ export const MONEY_PATH_TOKENS = ['cost', 'costs', 'spend', 'spending', 'budget'
|
||||
export const MONEY_PAIRS: readonly (readonly [string, readonly string[]])[] = [
|
||||
['usage', ['token', 'tokens', 'cost', 'costs', 'spend', 'billing', 'price', 'pricing', 'budget']], ['proxy', ['billing', 'bill']], ['client', ['cost', 'costs', 'charge', 'charges', 'billing', 'price', 'pricing', 'spend']],
|
||||
];
|
||||
/** Identifiers ending in _usd, Usd or cap_usd, any spend_cap spelling, and paid-provider call sites (the providers lib/pricing.ts prices). */
|
||||
export const MONEY_LINE = [
|
||||
/\b[A-Za-z0-9_]*_usd\b/i, /\b[a-z][A-Za-z0-9]*Usd\b/, /\bspend[_-]?cap\b/i,
|
||||
/** Identifiers ending in _usd or Usd: money only beside a word in MONEY_USD_WORDS or in a comparison (usdMoney). */
|
||||
export const MONEY_USD = [/\b[A-Za-z0-9_]*_usd\b/i, /\b[a-z][A-Za-z0-9]*Usd\b/];
|
||||
const MONEY_USD_ALL = MONEY_USD.map(r => new RegExp(r.source, r.flags + 'g'));
|
||||
/** What makes a _usd identifier money (GST-962: reading or reporting gstack-evals#139's per-trial cost_usd is not money code). */
|
||||
export const MONEY_USD_WORDS = ['cap', 'caps', 'limit', 'limits', 'max', 'maximum', 'budget', 'budgets', 'ceiling', 'allowance', 'spend', 'spent', 'spending', 'price', 'prices', 'pricing', 'fee', 'fees', 'bill', 'billing', 'paid', 'payment'];
|
||||
/** Any spend_cap spelling and paid-provider call sites (the providers lib/pricing.ts prices). */
|
||||
export const PAID_MONEY_LINE = [
|
||||
/\bspend[_-]?cap\b/i,
|
||||
/\bapi\.(?:anthropic|openai|voyageai)\.com\b/, /\bgenerativelanguage\.googleapis\.com\b/, /\bopenrouter\.ai\b/,
|
||||
/\bnew\s+(?:Anthropic|OpenAI|AnthropicBedrock|AnthropicVertex)\s*\(/, /\.(?:messages|chat\.completions|responses|embeddings)\.create\s*\(/,
|
||||
];
|
||||
export const MONEY_LINE = [...MONEY_USD, ...PAID_MONEY_LINE];
|
||||
/** A _usd identifier whose name says cap, budget, limit, spend or price, or that a `<` or `>` compares; never one an arrow reads. */
|
||||
export const usdMoney = (line: string) => MONEY_USD_ALL.some(r => [...line.matchAll(r)].some(m =>
|
||||
tokens(m[0]).some(x => MONEY_USD_WORDS.includes(x))
|
||||
|| /^\s{0,20}[<>]/.test(line.slice(m.index + m[0].length, m.index + m[0].length + 21))
|
||||
|| /(?<![=-])[<>]=?\s{0,20}(?:[\w$]{1,100}\.){0,10}$/.test(line.slice(Math.max(0, m.index - 300), m.index))));
|
||||
export const AUTH_PATH_TOKENS = ['auth', 'authn', 'authz', 'oauth', 'oauth2', 'jwt', 'session', 'sessions', 'permission', 'permissions', 'role', 'roles'];
|
||||
export const AUTH_PATH_PREFIXES = ['authenticat', 'authoriz'];
|
||||
export const SECRET_PATH = /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i;
|
||||
@@ -82,7 +93,7 @@ const namesMoney = (t: readonly string[], words: readonly string[]) => t.some(x
|
||||
/** A hash of the patterns that decide the risk classes, not of this file's bytes; lib/review-depth.ts folds it into DEPTH_RULES. */
|
||||
export const RISK_RULES = createHash('sha256').update(JSON.stringify({
|
||||
RISK_CLASSES, MONEY_PATH_TOKENS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME, SECURITY_PATH_TOKENS, DATA_PATH_TOKENS,
|
||||
MONEY_LINE_TOKENS, MONEY_PAIRS,
|
||||
MONEY_LINE_TOKENS, MONEY_PAIRS, MONEY_USD_WORDS,
|
||||
patterns: Object.entries({ MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH], SECURITY_LINE, SECURITY_JOINED, DATA_LINE, SVG_PATH: [SVG_PATH], SVG_ACTIVE }).map(([k, v]) => [k, v.map(r => '/' + r.source + '/' + r.flags)]),
|
||||
})).digest('hex').slice(0, 16);
|
||||
|
||||
@@ -95,6 +106,8 @@ export interface RiskInput {
|
||||
removed?: Record<string, readonly string[]>;
|
||||
/** gstack-diff-scope's SCOPE_AUTH: the auth class needs it and an auth token in the path. */
|
||||
scopeAuth: boolean;
|
||||
/** Code paths no code reads (an unconsumed data file or drawing): nothing runs them, so neither their name nor their rows name money (GST-961). */
|
||||
dataPaths?: readonly string[];
|
||||
}
|
||||
|
||||
/** The risk classes the code paths touch, the first reason per class, in RISK_CLASSES order. Nothing fails closed here: input errors are the caller's. */
|
||||
@@ -105,12 +118,13 @@ export function matchRisk(i: RiskInput): Map<RiskClass, string> {
|
||||
const t = tokens(p);
|
||||
if (i.scopeAuth && t.some(x => AUTH_PATH_TOKENS.includes(x) || AUTH_PATH_PREFIXES.some(a => x.startsWith(a)))) hit('auth', 'auth path: ' + p);
|
||||
if (SECRET_PATH.test(p)) hit('secret', 'secret path: ' + p);
|
||||
if (namesMoney(t, MONEY_PATH_TOKENS)) hit('money', 'money path: ' + p);
|
||||
const money = !i.dataPaths?.includes(p);
|
||||
if (money && namesMoney(t, MONEY_PATH_TOKENS)) hit('money', 'money path: ' + p);
|
||||
if (t.some(x => SECURITY_PATH_TOKENS.includes(x))) hit('security', 'security path: ' + p);
|
||||
if (t.some(x => DATA_PATH_TOKENS.includes(x))) hit('data', 'data path: ' + p);
|
||||
if (SVG_PATH.test(p) && [i.added?.[p], i.removed?.[p]].some(l => l && svgActive(l.join('\n')))) hit('security', 'active SVG lines: ' + p);
|
||||
for (const line of [...(i.added?.[p] ?? []), ...(i.removed?.[p] ?? [])]) {
|
||||
if (!why.has('money') && (MONEY_LINE.some(r => r.test(line)) || namesMoney(tokens(line), MONEY_LINE_TOKENS))) hit('money', 'money line: ' + p);
|
||||
if (money && !why.has('money') && (PAID_MONEY_LINE.some(r => r.test(line)) || usdMoney(line) || namesMoney(tokens(line), MONEY_LINE_TOKENS))) hit('money', 'money line: ' + p);
|
||||
if (!why.has('secret') && SECRET_LINE.some(r => r.test(line))) hit('secret', 'secret line: ' + p);
|
||||
if (!why.has('security') && SECURITY_LINE.some(r => r.test(line))) hit('security', 'security line: ' + p);
|
||||
if (!why.has('data') && DATA_LINE.some(r => r.test(line))) hit('data', 'data line: ' + p);
|
||||
|
||||
@@ -20,8 +20,8 @@ import { spawnSync } from 'node:child_process';
|
||||
import { createHash } from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { classifyPath } from './review-depth';
|
||||
import { AUTH_PATH_PREFIXES, AUTH_PATH_TOKENS, KEY_NAME, MONEY_LINE, MONEY_LINE_TOKENS, MONEY_PAIRS, MONEY_PATH_TOKENS, SECRET_LINE, SECRET_PATH, matchRisk } from './review-risk';
|
||||
import { classifyPath, unconsumedData, type ClassInput } from './review-depth';
|
||||
import { AUTH_PATH_PREFIXES, AUTH_PATH_TOKENS, KEY_NAME, MONEY_LINE, MONEY_LINE_TOKENS, MONEY_PAIRS, MONEY_PATH_TOKENS, MONEY_USD_WORDS, SECRET_LINE, SECRET_PATH, matchRisk } from './review-risk';
|
||||
|
||||
export const TARGETED_CLASSES = ['money', 'gate', 'auth', 'secret'] as const;
|
||||
export type TargetedClass = (typeof TARGETED_CLASSES)[number];
|
||||
@@ -31,7 +31,7 @@ export const secretLine = (line: string) => SECRET_LINE.some(r => r.test(line));
|
||||
|
||||
/** A hash of the patterns that decide the classes, not of this file's bytes: a comment edit keeps it, a pattern edit changes it. */
|
||||
export const TARGETED_RULES = createHash('sha256').update(JSON.stringify({
|
||||
TARGETED_CLASSES, MONEY_PATH_TOKENS, MONEY_LINE_TOKENS, MONEY_PAIRS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME,
|
||||
TARGETED_CLASSES, MONEY_PATH_TOKENS, MONEY_LINE_TOKENS, MONEY_PAIRS, MONEY_USD_WORDS, AUTH_PATH_TOKENS, AUTH_PATH_PREFIXES, KEY_NAME,
|
||||
patterns: Object.entries({ MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]),
|
||||
})).digest('hex').slice(0, 16);
|
||||
|
||||
@@ -47,6 +47,9 @@ export interface TargetedInput {
|
||||
root?: string | readonly string[];
|
||||
/** Collector failures (a diff git could not list). */
|
||||
errors?: readonly { code: string; input: string }[];
|
||||
/** The collector's consumer scan and drawings (ClassInput): a data file no code reads names no money. */
|
||||
consumed?: ClassInput['consumed'];
|
||||
drawings?: ClassInput['drawings'];
|
||||
}
|
||||
export interface TargetedResult { classes: TargetedClass[]; reasons: string[]; errors: string[]; rules: string }
|
||||
|
||||
@@ -61,7 +64,7 @@ export function targetedClasses(i: TargetedInput): TargetedResult {
|
||||
if (errors.length) return { classes: [...TARGETED_CLASSES], reasons: errors.map(e => `fail-closed: ${e}`), errors, rules: TARGETED_RULES };
|
||||
const why = new Map<TargetedClass, string>();
|
||||
if ((i.gateTagged ?? 0) > 0) why.set('gate', `gate scan tagged ${i.gateTagged} hunk(s)`);
|
||||
for (const [c, reason] of matchRisk({ codePaths: code, added: i.added, removed: i.removed, scopeAuth: s!.SCOPE_AUTH === 'true' })) if ((TARGETED_CLASSES as readonly string[]).includes(c)) why.set(c as TargetedClass, reason);
|
||||
for (const [c, reason] of matchRisk({ codePaths: code, dataPaths: code.filter(p => unconsumedData(i, p)), added: i.added, removed: i.removed, scopeAuth: s!.SCOPE_AUTH === 'true' })) if ((TARGETED_CLASSES as readonly string[]).includes(c)) why.set(c as TargetedClass, reason);
|
||||
const classes = TARGETED_CLASSES.filter(c => why.has(c));
|
||||
return { classes, reasons: classes.map(c => why.get(c)!), errors: [], rules: TARGETED_RULES };
|
||||
}
|
||||
|
||||
@@ -74,6 +74,7 @@ const NO_LOSS_REPLAYS = {
|
||||
'GST-967': "GST-967 dispatch-superset no-loss check (test/review-depth.test.ts, 'GST-967 dispatch-superset no-loss check'): over 1,440 planReview cases the branch plans exactly what main planned at 1022f4f except the 71 gate-class stops before Step 11, which add only Step 11's adversarial, outside and targeted voices, so no reviewer main dispatches is dropped or re-efforted; the brief bodies lib/review-brief.ts cuts from the ship sections hash to main's; only the lead's repeat-decision and Step 11 instructions changed; reviewer text is unchanged",
|
||||
'GST-508+GST-464+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: ship/sections/review-army.md and adversarial.md are main's GST-967 bytes plus exactly the GST-508+GST-464 lines (the merged file minus main equals the wave minus fc292e22f, line for line), so GST-967's dispatch-superset no-loss check and GST-508's no-loss replay both hold; merged-head checks in GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md",
|
||||
'GST-508+R51+GST-911+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: lib/review-depth.ts is main's GST-967 bytes plus exactly the GST-508+R51+GST-911 lines (the merged file minus main equals the wave minus fc292e22f, line for line); GST-967's dispatch-superset no-loss check (test/review-depth.test.ts) passes on the merged file, and the GST-508+R51+GST-911 checks are rerun at the merged head (GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md)",
|
||||
'GST-961+962': "GST-961+962 verdict no-loss replay: a data file no code reads (the depth's consumer scan) names no money by its path or rows, and a _usd or Usd name is money only when it says cap, limit, max, budget, spend, price, fee, bill or payment or a < or > compares it; over the last 100 first-parent main commits to bf2bcf14b, main's code against this branch, gate scan fixed at 0, no commit changes depth, class, risk classes or targeted classes (~/.capy/work/gst898/replay961.ts on GSTA-249's machine); the 76 changed lines that no longer read as money lines are cost reads and reports (paid-report, eval-plan projections, autoplan estimates), tests, docs and the old pattern source; lib/review-depth.ts only exports unconsumedData; reviewer text is unchanged",
|
||||
'GST-940': "GST-940 verdict no-loss check: svgDrawing reads a style element's opening tag as quoted pairs with ASCII whitespace, walks its body over the raw text, refuses any & in it, reads comments as CSS and refuses a non-ASCII element or attribute name; on 62,805 unique SVG files (21,658 with a style element) it gives the same verdict as main at 4bbad2422 on every file, and the planner over the last 100 first-parent main commits to 4bbad2422 changes no commit's depth, class or risk classes; the only verdicts it changes are the GST-940 cases in test/review-planner-misreads.test.ts; rerun on the merge with main at 8f08cb76a (GST-967 merged), with the same results; rerun on the merge with main at 5f705f367 (#3852 and #3864 merged), with the same results",
|
||||
} as const;
|
||||
type Replay = keyof typeof NO_LOSS_REPLAYS;
|
||||
@@ -96,8 +97,8 @@ const REVIEWER_BRIEFS: Record<string, { sha: string; replay: Replay }> = {
|
||||
'review/specialists/testing.md': { sha: 'd6cc2bef7be562387d0c0a22fefe646dff9c295f1d34d8453a6cb496584150e4', replay: '#3638' },
|
||||
'review/specialists/targeted.md': { sha: '7f92a0d0a8319c3c0e076fb3b1a4689c45120b2be2c14dfc7d4cb03aafa9d6a5', replay: 'GST-257' },
|
||||
'scripts/resolvers/fix-check.ts': { sha: '712bf5b0c439e96bac32f19cbe25394dabb4c94aa66600100bfde9b100e15a08', replay: '#3638' },
|
||||
'lib/review-depth.ts': { sha: '25050a7a61f81df649d1df0d0237dbf961671356395317c6a527741f5bb56877', replay: 'GST-940' },
|
||||
'lib/review-risk.ts': { sha: '08464b78a723ee463ca3a28d0abbc4eb8f4f512192061c703743707b66b5a311', replay: 'GST-942' },
|
||||
'lib/review-depth.ts': { sha: '894ef79782232ae8e2b13a25d57d87264125ff6ae5aeb70813c4b11c8cd59a4d', replay: 'GST-961+962' },
|
||||
'lib/review-risk.ts': { sha: 'ac023de9073705a31f864bc9dc9a58262c325185375dff9049c917fbe8d2be76', replay: 'GST-961+962' },
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { SVG_REFUSED, svgDrawing } from '../lib/review-depth';
|
||||
import { DATA_LINE, MONEY_LINE, SECRET_LINE, SECRET_PATH, SECURITY_JOINED, SECURITY_LINE, matchRisk } from '../lib/review-risk';
|
||||
import { DATA_LINE, MONEY_LINE, SECRET_LINE, SECRET_PATH, SECURITY_JOINED, SECURITY_LINE, matchRisk, usdMoney } from '../lib/review-risk';
|
||||
|
||||
const L = 100_000;
|
||||
const ms = (f: () => unknown) => { const t = performance.now(); f(); return performance.now() - t; };
|
||||
@@ -37,6 +37,11 @@ describe('linear time on hostile input', () => {
|
||||
expect(ms(() => { hit = security(line); })).toBeLessThan(250);
|
||||
expect(hit).toBe(false);
|
||||
}, 60_000);
|
||||
test('usdMoney (GST-962) returns in under 250 ms on each 100k-character line of _usd names, comparisons and member chains', () => {
|
||||
const lines = [run('', 'x_usd '), run('', 'aUsd '), run('', '> a.x_usd '), run('', '=> t.cost_usd '), run('>', ' '), run('> ', 'a.', 'x_usd'), run('', 'a_', 'usd'), run('', '<'), run('', 'cost_usd>')];
|
||||
const slow = lines.map(([label, s]) => [label, Math.round(ms(() => usdMoney(s)))] as const).filter(([, t]) => t >= 250);
|
||||
expect(slow).toEqual([]);
|
||||
}, 60_000);
|
||||
});
|
||||
|
||||
describe('the rm rule matches what R51 matched', () => {
|
||||
|
||||
Reference in new issue
Block a user