mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-04 15:57:33 +01:00
fix(review): a drawing-only SVG is data and a scripted one is security; session data files are not auth; diff-scope and the receipt depth read origin/<base> by full ref name (GST-911, GST-884)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
f254cebf12
commit
8b034914cc
10 files changed
+169
-31
No files matched your search
+47
-6
@@ -37,6 +37,30 @@ BASE="${1:-$(_default_base)}"
|
||||
HEAD_REF=""
|
||||
if [ "${2:-}" = "--head" ]; then HEAD_REF="${3:-}"; fi
|
||||
|
||||
# The base as a commit (GST-884, mirroring lib/gate-diff/git.ts resolveBase from
|
||||
# GST-883): a branch name reads refs/remotes/<base> (origin/main as given), then
|
||||
# refs/remotes/<remote>/<base> that shares history with the head (origin first),
|
||||
# then refs/heads/<base>, all by full ref name, so a stale local main never widens
|
||||
# the scope and a tag named main or origin/main cannot stand in for the branch.
|
||||
# A sha, a relative ref (HEAD~1, main^2, @{u}) or a full refs/ name resolves as
|
||||
# given. Prints nothing when nothing resolves.
|
||||
_is_branch_name() {
|
||||
case "$1" in refs/*|*~*|*^*|*@\{*|HEAD|*_HEAD) return 1 ;; esac
|
||||
[[ ! "$1" =~ ^[0-9a-f]{7,64}$ ]]
|
||||
}
|
||||
_commit() { git rev-parse --verify -q "$1^{commit}" 2>/dev/null; }
|
||||
_resolve_base() {
|
||||
local ref="$1" head="$2" sha remote
|
||||
if ! _is_branch_name "$ref"; then _commit "$ref"; return; fi
|
||||
sha=$(_commit "refs/remotes/$ref") && { echo "$sha"; return; }
|
||||
for remote in $(git remote 2>/dev/null | grep -x origin; git remote 2>/dev/null | grep -vx origin); do
|
||||
sha=$(_commit "refs/remotes/$remote/$ref") || continue
|
||||
git merge-base "$sha" "$head" >/dev/null 2>&1 && { echo "$sha"; return; }
|
||||
done
|
||||
sha=$(_commit "refs/heads/$ref") && { echo "$sha"; return; }
|
||||
_commit "$ref"
|
||||
}
|
||||
|
||||
FRONTEND=false
|
||||
BACKEND=false
|
||||
PROMPTS=false
|
||||
@@ -53,6 +77,19 @@ AUTH=false
|
||||
# the root-anchored globs alone (#3335).
|
||||
TEST_PATH_RE='(^|/)(tests?|spec|__tests__)/|(^|/)tests?\.py$|(^|/)test_[^/]+\.py$|_test\.(go|py|rb|ts|js|exs|dart)$|\.(test|spec)\.[jt]sx?$|_spec\.rb$|Test\.(java|kt)$'
|
||||
|
||||
# Data files (lib/review-depth.ts DATA_PATH) outside the config locations its
|
||||
# DATA_CONFIG and agent-prose rules keep code: .github/, config/, dotfiles,
|
||||
# *.config.*, .gstack/. Case-insensitive, as DATA_PATH is.
|
||||
DATA_FILE_RE='\.(json|jsonl|ndjson|csv|tsv|png|jpe?g|gif|webp|bmp|ico|avif)$'
|
||||
DATA_CONFIG_RE='(^|/)\.github/|(^|/)config/|(^|/)\.[^/]+$|(^|/)[^/]+\.config\.[^/]+$|^\.gstack/'
|
||||
_session_data() {
|
||||
local r=1
|
||||
shopt -s nocasematch
|
||||
if [[ "$1" =~ $DATA_FILE_RE ]] && [[ ! "$1" =~ $DATA_CONFIG_RE ]]; then r=0; fi
|
||||
shopt -u nocasematch
|
||||
return "$r"
|
||||
}
|
||||
|
||||
_print_flags() {
|
||||
echo "SCOPE_FRONTEND=$FRONTEND"
|
||||
echo "SCOPE_BACKEND=$BACKEND"
|
||||
@@ -68,7 +105,8 @@ _print_flags() {
|
||||
# Base reachability (#2526): a shallow CI checkout or an unfetched ref makes
|
||||
# `git diff` return an empty list — all-false with exit 0, a green that means
|
||||
# "we could not look". Distinguish it before diffing.
|
||||
if ! git rev-parse --verify -q "${BASE}^{commit}" >/dev/null 2>&1; then
|
||||
BASE_SHA=$(_resolve_base "$BASE" "${HEAD_REF:-HEAD}" || true)
|
||||
if [ -z "$BASE_SHA" ]; then
|
||||
_print_flags
|
||||
echo "SCOPE_ERROR=no_base"
|
||||
echo "# base ref '${BASE}' is not resolvable — shallow checkout or missing fetch. Run: git fetch origin ${BASE}"
|
||||
@@ -91,16 +129,16 @@ if [ -n "$HEAD_REF" ] || [ "${2:-}" = "--head" ]; then
|
||||
echo "# --head '${HEAD_REF}' is not a commit in this repository"
|
||||
exit 2
|
||||
fi
|
||||
if ! git merge-base "${BASE}" "${HEAD_REF}" >/dev/null 2>&1; then
|
||||
if ! git merge-base "${BASE_SHA}" "${HEAD_REF}" >/dev/null 2>&1; then
|
||||
_print_flags
|
||||
echo "SCOPE_ERROR=no_merge_base"
|
||||
echo "# '${BASE}' and '${HEAD_REF}' share no merge base"
|
||||
exit 2
|
||||
fi
|
||||
_collect < <(git diff -z "${BASE}...${HEAD_REF}" --name-only 2>/dev/null)
|
||||
_collect < <(git diff -z "${BASE_SHA}...${HEAD_REF}" --name-only 2>/dev/null)
|
||||
else
|
||||
# Committed diff vs base (merge-base form; two-dot fallback when no merge base).
|
||||
_collect < <(git diff -z "${BASE}...HEAD" --name-only 2>/dev/null || git diff -z "${BASE}" --name-only 2>/dev/null || true)
|
||||
_collect < <(git diff -z "${BASE_SHA}...HEAD" --name-only 2>/dev/null || git diff -z "${BASE_SHA}" --name-only 2>/dev/null || true)
|
||||
# Working-tree changes (staged + unstaged). `git diff HEAD` fails on a repo
|
||||
# with no commits; tolerated.
|
||||
_collect < <(git diff -z HEAD --name-only 2>/dev/null || true)
|
||||
@@ -190,9 +228,12 @@ for f in ${FILES_LIST[@]+"${FILES_LIST[@]}"}; do
|
||||
*.graphql|*.gql|openapi.*|swagger.*) m_api=true ;;
|
||||
esac
|
||||
|
||||
# Auth: authentication, authorization, sessions, permissions
|
||||
# Auth: authentication, authorization, sessions, permissions. A data file
|
||||
# named for sessions (recovered-sessions.jsonl, GST-911) holds rows, not
|
||||
# session handling, so it is not auth; session config still is.
|
||||
case "$f" in
|
||||
*auth*|*session*|*jwt*|*oauth*|*permission*|*role*) m_auth=true ;;
|
||||
*auth*|*jwt*|*oauth*|*permission*|*role*) m_auth=true ;;
|
||||
*session*) _session_data "$f" || m_auth=true ;;
|
||||
esac
|
||||
|
||||
# Backend: code that isn't a frontend component/view file. Includes ESM/CJS
|
||||
|
||||
@@ -149,7 +149,7 @@ function main(): number {
|
||||
const gateScan = { tagged: gateTagged, head };
|
||||
if (has('--json')) {
|
||||
const { runtimeLines, stepDepth, depthReasons, ...rest } = plan;
|
||||
process.stdout.write(JSON.stringify({ ...rest, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed }) + '\n');
|
||||
process.stdout.write(JSON.stringify({ ...rest, targeted: { ...targeted, brief, targeted_patch_id: patch }, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, fix_range: fixRange ?? null, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly, consumed: collected.consumed, drawings: collected.drawings }) + '\n');
|
||||
return EXIT.ok;
|
||||
}
|
||||
const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`,
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
### Fixed
|
||||
|
||||
- **/ship no longer reviews a results write-up as if it were gate code.** Three planner misreads on gstack-evals#132 (GST-911) pushed a docs PR to the five-reader full panel. A chart SVG made of drawing elements now counts as data, like the same chart as a PNG; an SVG with a script, `foreignObject`, event handler or `javascript:` URL stays code and now gets the security class. A data file named for sessions (`recovered-sessions.jsonl`) no longer sets the auth scope, so it adds no auth reader and no gate class; session code and session config still do.
|
||||
- **`gstack-diff-scope` and the receipt depth read `origin/<base>` by its full ref name (GST-884).** A local `main` behind `origin/main` no longer adds upstream files to the scope, and a tag named `main` or `origin/main` can no longer stand in for the branch.
|
||||
|
||||
### Tests
|
||||
|
||||
- `test/review-planner-misreads.test.ts`: a 600-point matplotlib-style chart plans docs, light, runtime 0 (before: code, 636 runtime lines); six ways an SVG can run script keep it code with the security class, and six other non-drawing constructs keep it code; session data files leave `SCOPE_AUTH` false while session code and config set it; a stale local `main` and tags named `main` or `origin/main` leave the scope and the receipt base unchanged.
|
||||
+14
-6
@@ -181,7 +181,12 @@ follows that file, four files deep. Precedence: agent prose (`.gstack/**`), a te
|
||||
fixture path, a gate path, a config file (`.github/**`, anything under a `config/`
|
||||
directory, dotfiles, `*.config.*`, `package.json`, lockfiles, `tsconfig*.json`,
|
||||
`manifest.json` and the other tool manifests `lib/review-depth.ts` lists) and a named
|
||||
consumer each keep the path code, and YAML, TOML and SVG are always code. More than
|
||||
consumer each keep the path code, and YAML and TOML are always code. An SVG counts
|
||||
as data only when its whole text at the head is a drawing (GST-911): SVG drawing
|
||||
elements and RDF metadata, CSS with no import and no `url()` outside the file, every
|
||||
`href` a same-file `#` reference or an inline raster image, no event handler and no
|
||||
entity declaration, read after character references are decoded; anything else, a
|
||||
script or `foreignObject` included, keeps it code. More than
|
||||
2,000 data paths keep every one code; a `git grep` failure is `consumer_scan_failed`
|
||||
and fails the depth closed. Only `classifyDiff` applies the rule (`lib/review-collect.ts`
|
||||
finds the consumers, `gstack-ship-preflight`'s `RUNTIME_LINES` reads the same
|
||||
@@ -203,15 +208,18 @@ added and deleted lines; any class makes the depth full (`risk:<class>` in
|
||||
[targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code)'s matchers.
|
||||
**security** is a trust-boundary path (trust, redact, sanitize, neutralize,
|
||||
consent, escape) or line (`shell: true`, `execSync(`, `eval`, `new Function(`,
|
||||
raw HTML sinks, a shell run through argv, recursive deletes). **data** is user data: a path naming pii, gdpr, ccpa, privacy,
|
||||
raw HTML sinks, a shell run through argv, recursive deletes), or a changed SVG
|
||||
line that can run code (a script or `foreignObject` element, an event handler, a
|
||||
`javascript:` URL). **data** is user data: a path naming pii, gdpr, ccpa, privacy,
|
||||
personal, account, customer, cookie, persistence, storage or database, or a line
|
||||
that drops a table or column, deletes rows or bulk-deletes records. Auth scope,
|
||||
migrations and deleted code paths were already gate class. `RISK_RULES` hashes
|
||||
the patterns and `DEPTH_RULES` includes it.
|
||||
|
||||
**UI text and SVGs** stay code (light unless a risk class applies). A docs path
|
||||
for UI copy, locale values and chart SVGs waits for an allowlist classifier: three
|
||||
rounds of R51 review found bypasses in line-pattern versions of both.
|
||||
**UI text** stays code (light unless a risk class applies). A docs path for UI
|
||||
copy and locale values waits for an allowlist classifier: three rounds of R51
|
||||
review found bypasses in line-pattern versions. Chart SVGs use the drawing
|
||||
allowlist above, which reads the whole file rather than changed lines.
|
||||
|
||||
**Batched checklists (the lean ensemble, GSTA-167 part 2).** One specialist
|
||||
subagent reads Testing and Security on every code or gate diff, plus
|
||||
@@ -330,7 +338,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 |
|
||||
| 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` |
|
||||
| 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
|
||||
|
||||
+25
-6
@@ -12,7 +12,7 @@ import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { GENERATED_PATH, parseRegistry } from './regen';
|
||||
import { RESULT_CODES } from './result-codes';
|
||||
import { classifyPath, dataCandidate, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth';
|
||||
import { classifyPath, dataCandidate, svgCandidate, svgDrawing, type ClassInput, type DepthError, type PathLines, type RegistryInfo } from './review-depth';
|
||||
|
||||
export interface Collected extends ClassInput {
|
||||
perPath: PathLines[];
|
||||
@@ -28,6 +28,7 @@ export interface Collected extends ClassInput {
|
||||
mergeBase: string | null;
|
||||
untracked: string[];
|
||||
consumed: string[] | 'all';
|
||||
drawings: string[];
|
||||
}
|
||||
|
||||
function git(root: string, args: string[]): { ok: boolean; status: number | null; out: string; err: string } {
|
||||
@@ -106,7 +107,7 @@ export const CONSUMER_SCAN_DEPTH = 4;
|
||||
* a collector error, which fails the depth closed.
|
||||
*/
|
||||
export function scanConsumers(root: string, paths: readonly string[], revs: readonly string[], live: boolean, classRoot: string | readonly string[], errors: DepthError[]): string[] | 'all' {
|
||||
const candidates = [...new Set(paths)].filter(p => dataCandidate(p, classRoot)).sort();
|
||||
const candidates = [...new Set(paths)].filter(p => dataCandidate(p, classRoot) || svgCandidate(p, classRoot)).sort();
|
||||
if (candidates.length > CONSUMER_SCAN_CAP) return 'all';
|
||||
const base = (p: string) => p.slice(p.lastIndexOf('/') + 1);
|
||||
const named = new Map<string, Set<string>>();
|
||||
@@ -146,9 +147,20 @@ export function scanConsumers(root: string, paths: readonly string[], revs: read
|
||||
return candidates.filter(p => consumed.has(p));
|
||||
}
|
||||
|
||||
function result(diff: ReturnType<typeof readDiff>, extra: Pick<Collected, 'root' | 'registry' | 'errors' | 'mergeBase' | 'untracked' | 'consumed'>): Collected {
|
||||
/** The largest SVG whose text is read for the drawing check; a larger one stays code. */
|
||||
export const DRAWING_READ_CAP = 8 * 1024 * 1024;
|
||||
|
||||
/** The changed SVG candidates whose whole text, read by `read` (null when it cannot be read), is a drawing (lib/review-depth.ts svgDrawing). */
|
||||
function drawingsAmong(perPath: readonly PathLines[], classRoot: string | readonly string[], read: (p: string) => Buffer | null): string[] {
|
||||
return perPath.filter(e => !e.binary && svgCandidate(e.path, classRoot)).map(e => e.path).filter(p => {
|
||||
const buf = read(p);
|
||||
return !!buf && buf.length <= DRAWING_READ_CAP && !buf.includes(0) && svgDrawing(buf.toString('utf8'));
|
||||
}).sort();
|
||||
}
|
||||
|
||||
function result(diff: ReturnType<typeof readDiff>, extra: Pick<Collected, 'root' | 'registry' | 'errors' | 'mergeBase' | 'untracked' | 'consumed'> & { drawings?: string[] }): Collected {
|
||||
const paths = diff.perPath.map(e => e.path).sort();
|
||||
return { ...diff, ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) };
|
||||
return { ...diff, drawings: [], ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) };
|
||||
}
|
||||
|
||||
/** The range `baseSha`...`headSha` from git objects alone; the registry is the base commit's, never the head's. */
|
||||
@@ -165,7 +177,11 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C
|
||||
const tree = git(root, ['ls-tree', '-r', '--name-only', '-z', headSha]);
|
||||
const skillDirs = [...new Set(tree.out.split('\0').flatMap(p => /^([^/]+)\/SKILL\.md(\.tmpl)?$/.exec(p)?.[1] ?? []))].sort();
|
||||
const consumed = scanConsumers(root, [...diff.perPath.map(e => e.path), ...diff.deleted], [headSha, mergeBase], false, skillDirs, errors);
|
||||
return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed });
|
||||
const drawings = drawingsAmong(diff.perPath, skillDirs, p => {
|
||||
const blob = spawnSync('git', ['cat-file', 'blob', `${headSha}:${p}`], { cwd: root, timeout: 60_000, maxBuffer: DRAWING_READ_CAP + 1 });
|
||||
return blob.status === 0 && !blob.error ? blob.stdout : null;
|
||||
});
|
||||
return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [], consumed, drawings });
|
||||
}
|
||||
|
||||
/** The working tree against the merge base with origin/<base> (or <base>), untracked files counted per path; the registry is the base ref's commit. */
|
||||
@@ -193,5 +209,8 @@ export function collectLive(root: string, base: string): Collected {
|
||||
if (!binary) diff.added[p] = text.split('\n');
|
||||
}
|
||||
const consumed = scanConsumers(root, [...diff.perPath.map(e => e.path), ...diff.deleted], [mergeBase], true, root, errors);
|
||||
return result(diff, { root, registry, errors, mergeBase, untracked, consumed });
|
||||
const drawings = drawingsAmong(diff.perPath, root, p => {
|
||||
try { return fs.statSync(path.join(root, p)).size <= DRAWING_READ_CAP ? fs.readFileSync(path.join(root, p)) : null; } catch { return null; }
|
||||
});
|
||||
return result(diff, { root, registry, errors, mergeBase, untracked, consumed, drawings });
|
||||
}
|
||||
+52
-5
@@ -13,7 +13,7 @@ import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import type { ResultCodeName } from './result-codes';
|
||||
import { matchesAny } from './ship-policy';
|
||||
import { matchRisk, RISK_RULES, type RiskClass } from './review-risk';
|
||||
import { decodeCharRefs, matchRisk, RISK_RULES, SVG_PATH, svgActive, type RiskClass } from './review-risk';
|
||||
|
||||
export type DiffClass = 'docs' | 'tests' | 'code' | 'gate';
|
||||
export type PathClass = 'docs' | 'tests' | 'code';
|
||||
@@ -58,7 +58,7 @@ const GATE_PATH = [
|
||||
/^lib\/review-(depth|collect|scope|targeted|risk)\.ts$/, /^review\/specialists\/targeted\.md$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/,
|
||||
];
|
||||
|
||||
/** Data files: JSON, CSV, TSV and raster images. YAML, TOML and SVG are never data here: deploy configs, API specs and executable SVG stay code. */
|
||||
/** Data files: JSON, CSV, TSV and raster images. YAML and TOML are never data here: deploy configs and API specs stay code. An SVG is data only as a drawing (svgDrawing). */
|
||||
export const DATA_PATH = /\.(json|jsonl|ndjson|csv|tsv|png|jpe?g|gif|webp|bmp|ico|avif)$/i;
|
||||
/** Data-shaped files tools read by convention, with no code naming them: always code. */
|
||||
const DATA_CONFIG = [
|
||||
@@ -66,10 +66,50 @@ const DATA_CONFIG = [
|
||||
/(^|\/)(package|package-lock|npm-shrinkwrap|composer|deno|jsr|tsconfig[^/]*|jsconfig[^/]*|manifest|app|vercel|netlify|firebase|renovate|turbo|nx|lerna|biome|angular|nest-cli|lint-staged|components|pyrightconfig|cdk|serverless|appsettings[^/]*)\.json$/i,
|
||||
];
|
||||
|
||||
/**
|
||||
* The SVG elements a chart is drawn with, lowercased, under any namespace prefix: shapes, text, paint servers, clip paths,
|
||||
* filters (feImage loads a document, so it is left out), CSS, and the RDF metadata plotting libraries write. No script,
|
||||
* foreignObject, link, animation or embedded document.
|
||||
*/
|
||||
const SVG_ELEMENTS = new Set([
|
||||
'svg', 'g', 'defs', 'symbol', 'use', 'path', 'rect', 'circle', 'ellipse', 'line', 'polyline', 'polygon', 'text', 'tspan', 'textpath', 'title', 'desc',
|
||||
'metadata', 'clippath', 'mask', 'pattern', 'marker', 'lineargradient', 'radialgradient', 'stop', 'style', 'image', 'filter',
|
||||
'feblend', 'fecolormatrix', 'fecomponenttransfer', 'fecomposite', 'feconvolvematrix', 'fediffuselighting', 'fedisplacementmap', 'fedistantlight',
|
||||
'fedropshadow', 'feflood', 'fefunca', 'fefuncb', 'fefuncg', 'fefuncr', 'fegaussianblur', 'femerge', 'femergenode', 'femorphology', 'feoffset',
|
||||
'fepointlight', 'fespecularlighting', 'fespotlight', 'fetile', 'feturbulence',
|
||||
'rdf', 'work', 'agent', 'type', 'date', 'format', 'creator', 'description', 'license',
|
||||
]);
|
||||
/** Markup a drawing may hold besides elements: comments, CDATA, the XML declaration, and a DOCTYPE with no internal subset. */
|
||||
const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg\b[^>[]*>)/i;
|
||||
/** What a drawing never holds: a named entity beyond the five XML ones, a CSS import, a url() to another document, a CSS expression or binding. */
|
||||
const SVG_REFUSED = [/&(?!(?:amp|lt|gt|quot|apos);)/, /@import/i, /url\(\s*(?!['"]?\s*#)/i, /expression\s*\(/i, /-moz-binding|\bbehavior\s*:/i];
|
||||
/** An href or src value a drawing may hold: a same-document reference, or a raster image inline. */
|
||||
const SVG_HREF = /^\s*(?:#|data:image\/(?:png|jpe?g|gif|webp);base64,[A-Za-z0-9+/=\s]*$)/i;
|
||||
|
||||
/**
|
||||
* Whether an SVG's whole text is a drawing (GST-911): every element in SVG_ELEMENTS, no event handler, every href or src a
|
||||
* same-document reference or an inline raster image, and nothing SVG_REFUSED names, read after character references are
|
||||
* decoded. Anything else, an SVG that can run code included, stays code.
|
||||
*/
|
||||
export function svgDrawing(text: string): boolean {
|
||||
const t = decodeCharRefs(text);
|
||||
if (SVG_REFUSED.some(r => r.test(t)) || t.split('\n').some(svgActive)) return false;
|
||||
for (let at = t.indexOf('<'); at !== -1; at = t.indexOf('<', at + 1)) {
|
||||
if (SVG_MARKUP.test(t.slice(at, at + 512))) continue;
|
||||
const tag = /^<\/?(?:[A-Za-z_][\w.-]*:)?([A-Za-z_][\w.-]*)/.exec(t.slice(at, at + 256));
|
||||
if (!tag || !SVG_ELEMENTS.has(tag[1]!.toLowerCase())) return false;
|
||||
}
|
||||
for (const m of t.matchAll(/(?<![\w.:-])([A-Za-z_][\w.:-]*)\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'>]+))/g)) {
|
||||
const local = m[1]!.slice(m[1]!.lastIndexOf(':') + 1).toLowerCase();
|
||||
if (local.startsWith('on') || ((local === 'href' || local === 'src') && !SVG_HREF.test(m[2] ?? m[3] ?? m[4] ?? ''))) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/** A hash of the constants and patterns that decide a depth, not of this file's bytes: a comment edit keeps it, a threshold edit changes it. */
|
||||
export const DEPTH_RULES = createHash('sha256').update(JSON.stringify({
|
||||
RISK_RULES, SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, BATCH_SCOPED, SCOPED_RANGE_LINES, ROLE_EFFORT,
|
||||
patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]),
|
||||
RISK_RULES, SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, BATCH_SCOPED, SCOPED_RANGE_LINES, ROLE_EFFORT, SVG_ELEMENTS: [...SVG_ELEMENTS],
|
||||
patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH], DATA_PATH: [DATA_PATH], DATA_CONFIG, SVG_MARKUP: [SVG_MARKUP], SVG_REFUSED, SVG_HREF: [SVG_HREF] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]),
|
||||
})).digest('hex').slice(0, 16);
|
||||
|
||||
/**
|
||||
@@ -97,6 +137,11 @@ export function dataCandidate(p: string, root?: string | readonly string[]): boo
|
||||
return DATA_PATH.test(p) && classifyPath(p, root) === 'code' && ![...AGENT_PROSE, ...GATE_PATH, ...DATA_CONFIG].some(r => r.test(p));
|
||||
}
|
||||
|
||||
/** An SVG the data rule may move to docs once the collector reads its whole text as a drawing (ClassInput.drawings); never a data file another file is named through. */
|
||||
export function svgCandidate(p: string, root?: string | readonly string[]): boolean {
|
||||
return SVG_PATH.test(p) && classifyPath(p, root) === 'code' && ![...AGENT_PROSE, ...GATE_PATH, ...DATA_CONFIG].some(r => r.test(p));
|
||||
}
|
||||
|
||||
/** Whether a range needs the gate scan: any path that is not docs by the path-only rule. The data rule never skips it. */
|
||||
export function gateScanNeeded(paths: readonly string[], root?: string | readonly string[]): boolean {
|
||||
return paths.some(p => classifyPath(p, root) !== 'docs');
|
||||
@@ -131,6 +176,8 @@ export interface ClassInput {
|
||||
* (lib/review-collect.ts). A candidate not listed counts as docs; 'all' or no scan keeps every data file code.
|
||||
*/
|
||||
consumed?: readonly string[] | 'all';
|
||||
/** The SVG candidates (svgCandidate) whose whole text the collector read as a drawing (svgDrawing); any other SVG stays code. */
|
||||
drawings?: readonly string[];
|
||||
/** Added and deleted lines per path (lib/review-collect.ts): the risk classes read them; without them no line signal applies. */
|
||||
added?: Record<string, readonly string[]>;
|
||||
removed?: Record<string, readonly string[]>;
|
||||
@@ -139,7 +186,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);
|
||||
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)));
|
||||
const pathClass = (i: ClassInput, p: string): PathClass => ((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. */
|
||||
|
||||
+13
-2
@@ -6,7 +6,8 @@
|
||||
* money, auth and secret are the targeted pass's matchers (GST-257). security is a trust-boundary path or line
|
||||
* (a redaction, sanitizer, consent or trust module; a shell, eval or raw HTML sink); data is user data (a path
|
||||
* naming personal or stored data, or a line that drops or bulk-deletes it). Path and identifier signals are
|
||||
* token-bounded: costume never names cost.
|
||||
* token-bounded: costume never names cost. A changed SVG line that can run code (a script or foreignObject element, an
|
||||
* event handler, a javascript: URL; character references decoded) is security (GST-911).
|
||||
*/
|
||||
import { createHash } from 'node:crypto';
|
||||
|
||||
@@ -40,6 +41,15 @@ const SECURITY_LINE = [
|
||||
/\brmSync\s*\([^)]*recursive\s*:\s*true/, /\brm\s+-(?:[a-z]*r[a-z]*f|[a-z]*f[a-z]*r)\b/,
|
||||
/\b(?:innerHTML|outerHTML)\s*\+?=/, /\bdangerouslySetInnerHTML\b/, /\bv-html\b/, /\{@html\b/, /\binsertAdjacentHTML\s*\(/, /\bdocument\.write\s*\(/,
|
||||
];
|
||||
export const SVG_PATH = /\.svg$/i;
|
||||
/** An SVG line that can run code, read decoded and again with whitespace removed (java	script: is javascript:). */
|
||||
export const SVG_ACTIVE = [/<(?:[\w.-]+:)?(?:script|foreignObject)\b/i, /(?:^|[\s"'/])on[a-z]+\s*=/i, /(?:java|vb)script:|data:text\/html/i];
|
||||
/** XML character references decoded (j and j are j); an out-of-range one decodes to nothing. */
|
||||
export const decodeCharRefs = (s: string) => s.replace(/&#(x[0-9a-f]{1,8}|\d{1,10});?/gi, (_, n: string) => {
|
||||
const c = n[0] === 'x' || n[0] === 'X' ? parseInt(n.slice(1), 16) : Number(n);
|
||||
return c <= 0x10ffff ? String.fromCodePoint(c) : '';
|
||||
});
|
||||
export const svgActive = (line: string) => { const d = decodeCharRefs(line); const squeezed = d.replace(/[\s\0-\x1f]+/g, ''); return SVG_ACTIVE.some(r => r.test(d) || r.test(squeezed)); };
|
||||
const DATA_PATH_TOKENS = ['pii', 'gdpr', 'ccpa', 'privacy', 'personal', 'account', 'accounts', 'customer', 'customers', 'cookie', 'cookies', 'persistence', 'storage', 'database'];
|
||||
const DATA_LINE = [
|
||||
/\bDELETE\s+FROM\b/, /\b(?:drop\s+(?:table|column|database|schema)|truncate\s+table)\b/i, /\bdelete\s+from\s+[\w."`]+\s*(?:where\b|;|$|["'`)])/i,
|
||||
@@ -49,7 +59,7 @@ const DATA_LINE = [
|
||||
/** 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,
|
||||
patterns: Object.entries({ MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH], SECURITY_LINE, DATA_LINE }).map(([k, v]) => [k, v.map(r => '/' + r.source + '/' + r.flags)]),
|
||||
patterns: Object.entries({ MONEY_LINE, SECRET_LINE, SECRET_PATH: [SECRET_PATH], SECURITY_LINE, DATA_LINE, SVG_PATH: [SVG_PATH], SVG_ACTIVE }).map(([k, v]) => [k, v.map(r => '/' + r.source + '/' + r.flags)]),
|
||||
})).digest('hex').slice(0, 16);
|
||||
|
||||
export const tokens = (p: string) => p.replace(/([a-z0-9])([A-Z])/g, '$1 $2').toLowerCase().split(/[^a-z0-9]+/).filter(Boolean);
|
||||
@@ -78,6 +88,7 @@ export function matchRisk(i: RiskInput): Map<RiskClass, string> {
|
||||
if (!why.has('money') && MONEY_LINE.some(r => r.test(line))) 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('security') && SVG_PATH.test(p) && svgActive(line)) hit('security', 'active SVG line: ' + p);
|
||||
if (!why.has('data') && DATA_LINE.some(r => r.test(line))) hit('data', 'data line: ' + p);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -117,7 +117,7 @@ function merge(cwd: string, base: string, r: Report): boolean {
|
||||
let runtime = 'unknown';
|
||||
try {
|
||||
const j = JSON.parse(plan.stdout);
|
||||
const c = classifyDiff({ paths: j.paths, perPath: j.per_path, stampOnly: j.stamp_only, registry: j.registry, consumed: j.consumed, root: cwd, gateTagged: 0 });
|
||||
const c = classifyDiff({ paths: j.paths, perPath: j.per_path, stampOnly: j.stamp_only, registry: j.registry, consumed: j.consumed, drawings: j.drawings, root: cwd, gateTagged: 0 });
|
||||
runtime = String(c.runtimeLines ?? 'unknown');
|
||||
} catch { /* unknown */ }
|
||||
r.add(`RUNTIME_LINES: ${runtime} (gstack-ship-budget check --runtime-lines at the Step 9 and 12 boundaries)`);
|
||||
|
||||
@@ -40,6 +40,9 @@ function git(root: string, args: string[]): { ok: boolean; out: string } {
|
||||
return { ok: r.status === 0, out: (r.stdout ?? '').trim() };
|
||||
}
|
||||
|
||||
/** A base as a commit; origin/<branch> reads refs/remotes/origin/<branch> by its full name, so a tag named origin/main cannot stand in for it (GST-884). */
|
||||
const baseCommit = (root: string, ref: string) => git(root, ['rev-parse', '--verify', '--quiet', `${/^origin\//.test(ref) ? `refs/remotes/${ref}` : ref}^{commit}`]);
|
||||
|
||||
/** gstack-diff-scope on the committed range `base`...`head` (`--head`): the same signals at write, carry and the queue. */
|
||||
function scopeOfRange(root: string, binDir: string, base: string, head: string): Record<string, string> {
|
||||
const sr = runScriptSync(path.join(binDir, 'gstack-diff-scope'), [base, '--head', head], { cwd: root, encoding: 'utf8', timeout: 60_000, maxBuffer: 16 * 1024 * 1024 });
|
||||
@@ -58,7 +61,7 @@ function scopeOfRange(root: string, binDir: string, base: string, head: string):
|
||||
*/
|
||||
export function receiptDepthAtHead(repoRoot: string, base: string, binDir: string, head: string): ReceiptDepth {
|
||||
const branch = base.replace(/^origin\//, '');
|
||||
const baseSha = git(repoRoot, ['rev-parse', '--verify', '--quiet', `origin/${branch}^{commit}`]);
|
||||
const baseSha = baseCommit(repoRoot, `origin/${branch}`);
|
||||
const baseRev = baseSha.ok ? baseSha.out : branch;
|
||||
const c = collectCommit(repoRoot, baseRev, head);
|
||||
const scope = scopeOfRange(repoRoot, binDir, baseRev, head);
|
||||
@@ -156,7 +159,7 @@ export interface DepthVerdict { ok: boolean; line: string; message?: string }
|
||||
/** The head's committed range against the base, read once for the depth verdict and the targeted line. */
|
||||
interface HeadInputs { baseSha: { ok: boolean; out: string }; c: ReturnType<typeof collectCommit>; scope: Record<string, string> }
|
||||
function headInputs(root: string, baseRef: string, head: string): HeadInputs {
|
||||
const baseSha = git(root, ['rev-parse', '--verify', '--quiet', `${baseRef}^{commit}`]);
|
||||
const baseSha = baseCommit(root, baseRef);
|
||||
const baseRev = baseSha.ok ? baseSha.out : baseRef;
|
||||
return { baseSha, c: collectCommit(root, baseRev, head), scope: scopeOfRange(root, BIN_DIR, baseRev, head) };
|
||||
}
|
||||
|
||||
@@ -56,6 +56,7 @@ const NO_LOSS_REPLAYS = {
|
||||
'G1': "GSTA-167 variant G Arm G1: live no-loss replay of both prose voices with part 1's prompts, 2 passes, normalizer v2, paired rule against pooled C' (parts 1 and 3; specialists at medium as in C and C2)",
|
||||
'L': "GSTA-167 variant G Arm L: live no-loss replay of one batched reader (testing and security plus scoped checklists) and the Red Team on its findings, 2 passes, scored L∘G1 against G1 on the paired rule (part 2)",
|
||||
'GST-257': "GST-257 PR 1 dispatch-superset no-loss check (test/review-targeted.test.ts): on every fixture diff the branch dispatches main's reviewers at 8be7f21 unchanged plus only the targeted reader, and over the last 100 non-release commits on main it lost none; adds the targeted role and brief to the G1 and L bytes, whose existing reviewer text is unchanged",
|
||||
'GST-911': "GST-911 no-loss replay (GSTA-249/13): the planner over the last 100 first-parent main commits to f5cb6ec1e, origin/main's code against this branch, gate scan fixed at 0, changes no commit's depth, class, risk classes, targeted classes or SCOPE_AUTH; the three misreads (a drawing-only SVG, a session-named data file, a bare base) are proven on fixtures (test/review-planner-misreads.test.ts), where every SVG that can run script stays code with the security class; reviewer text is unchanged",
|
||||
'R51': "R51 risk-tier routing no-loss re-score (GSTA-167): the saved panel findings (349 records over 24 of the 26 no-loss units) re-scored against the branch's plan lose no blocker whose only finders are roles it drops, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel (test/review-targeted.test.ts, test/review-risk-routing.test.ts); reviewer text is unchanged",
|
||||
} as const;
|
||||
type Replay = keyof typeof NO_LOSS_REPLAYS;
|
||||
@@ -78,8 +79,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: 'd649569a163c03476c6e0c5ea295f5d02088f96aab85fbc7d558f2ac2b27006a', replay: 'R51' },
|
||||
'lib/review-risk.ts': { sha: 'dbef7475d467e17340b21a2bacb52a16a7ccb1ee66291900ed2d36dfb5cfb7f2', replay: 'R51' },
|
||||
'lib/review-depth.ts': { sha: '7352b9e6e4e9f44c6e94f56cbb908d9a16b2ff536548770470af6169ffabccf7', replay: 'GST-911' },
|
||||
'lib/review-risk.ts': { sha: 'c1b1baec718798df9e7629d641a864dc2125455f51339535f7c95057e0d83fcd', replay: 'GST-911' },
|
||||
};
|
||||
|
||||
describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => {
|
||||
|
||||
Reference in new issue
Block a user