mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
fix: pre-landing review fixes (wave review-2, pass 1)
- bin/gstack-diff-scope: _resolve_base takes a related local <base> before another remote's twin, the order resolveBase uses, so the two tools pick the same base for a bare name when origin/<base> is unrelated. - lib/review-depth.ts: a drawing's DOCTYPE names no document but W3C's SVG DTD; SYSTEM and other PUBLIC identifiers keep the SVG code. Repin lib/review-depth.ts under no-loss replay GST-911-redos (the DOCTYPE change only keeps more SVGs code, so it drops no reviewer). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
b3d0eb8ad8
commit
86d0414fc1
5 files changed
+28
-11
No files matched your search
@@ -53,8 +53,9 @@ 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
|
||||
# refs/remotes/origin/<base> that shares history with the head, then refs/heads/<base>
|
||||
# that does, then another remote's twin that does, then refs/heads/<base> as it is,
|
||||
# all by full ref name and in resolveBase's order, 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.
|
||||
@@ -64,14 +65,15 @@ _is_branch_name() {
|
||||
}
|
||||
_commit() { git rev-parse --verify -q "$1^{commit}" 2>/dev/null; }
|
||||
_resolve_base() {
|
||||
local ref="$1" head="$2" sha remote
|
||||
local ref="$1" head="$2" sha loc 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
|
||||
loc=$(_commit "refs/heads/$ref")
|
||||
for remote in $(git remote 2>/dev/null | grep -x origin) ${loc:+:local} $(git remote 2>/dev/null | grep -vx origin); do
|
||||
if [ "$remote" = :local ]; then sha=$loc; else sha=$(_commit "refs/remotes/$remote/$ref") || continue; fi
|
||||
git merge-base "$sha" "$head" >/dev/null 2>&1 && { echo "$sha"; return; }
|
||||
done
|
||||
sha=$(_commit "refs/heads/$ref") && { echo "$sha"; return; }
|
||||
[ -n "$loc" ] && { echo "$loc"; return; }
|
||||
_commit "$ref"
|
||||
}
|
||||
|
||||
|
||||
+2
-2
@@ -79,8 +79,8 @@ const SVG_ELEMENTS = new Set([
|
||||
'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;
|
||||
/** Markup a drawing may hold besides elements: comments, CDATA, the XML declaration, and a DOCTYPE that names no document but W3C's SVG DTD. */
|
||||
const SVG_MARKUP = /^<(?:!--|!\[CDATA\[|\?xml\s|!DOCTYPE\s+svg(?:\s+PUBLIC\s+(["'])-\/\/W3C\/\/DTD SVG [\w .]+\/\/EN\1\s+(["'])https?:\/\/www\.w3\.org\/[\w.\/-]+\.dtd\2)?\s*>)/i;
|
||||
/**
|
||||
* What a drawing never holds: a named entity beyond the five XML ones, a backslash (a CSS escape such as @\69mport), a CSS
|
||||
* import, a url() to another document, a CSS expression or binding.
|
||||
|
||||
@@ -9,6 +9,7 @@ import { mkdtempSync, writeFileSync, mkdirSync, rmSync, readFileSync } from 'fs'
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { spawnSync } from 'child_process';
|
||||
import { resolveBase } from '../lib/gate-diff/git';
|
||||
|
||||
const SCRIPT = join(import.meta.dir, '..', 'bin', 'gstack-diff-scope');
|
||||
|
||||
@@ -380,6 +381,18 @@ describe('a bare base name reads origin/<base> when it exists (#3650)', () => {
|
||||
expect(scopeAt(dir, 'main').SCOPE_BACKEND).toBe('true');
|
||||
expect(upstreamCommit).not.toBe('');
|
||||
});
|
||||
|
||||
test('with origin/main unrelated, a related local main comes before a related other remote, the base resolveBase picks', () => {
|
||||
const { dir, git } = behindBase('main');
|
||||
git('remote', 'add', 'upstream', 'file:///nonexistent-upstream');
|
||||
git('update-ref', 'refs/remotes/upstream/main', git('rev-parse', 'origin/main'));
|
||||
git('checkout', '-q', '--orphan', 'island');
|
||||
git('commit', '-q', '--allow-empty', '-m', 'unrelated');
|
||||
git('update-ref', 'refs/remotes/origin/main', 'HEAD');
|
||||
git('checkout', '-q', 'feature/test');
|
||||
expect(resolveBase('main', dir, 'HEAD')?.sha).toBe(git('rev-parse', 'main'));
|
||||
expect(scopeAt(dir, 'main').SCOPE_BACKEND).toBe('true');
|
||||
});
|
||||
});
|
||||
|
||||
describe('uncommitted work is visible (#2299)', () => {
|
||||
|
||||
@@ -57,7 +57,7 @@ const NO_LOSS_REPLAYS = {
|
||||
'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-850': "GST-850 dispatch-superset no-loss check: on the 26 GSTA-167 no-loss units the branch plan (wider security and money classes, write-up text in the batched diff) drops no reviewer role main dispatches, so no saved blocker can be lost, and over the last 100 main commits no commit with a gate or risk class drops depth (test/review-risk-routing.test.ts, test/review-risk-linear.test.ts); reviewer text is unchanged",
|
||||
'GST-911-redos': "GST-911 ReDoS fix verdict-identity no-loss check (wave review-e7b551ebd865): SVG_REFUSED's url() entry, rewritten to run in linear time, refuses exactly the strings #3838's pattern refused (test/review-risk-redos.test.ts: 60,000 seeded strings and the hand-picked edges, and every SVG blob in the repository's history, with zero differences), so GST-911's no-loss replay holds unchanged; reviewer text is unchanged",
|
||||
'GST-911-redos': "GST-911 ReDoS fix verdict-identity no-loss check (wave review-e7b551ebd865): SVG_REFUSED's url() entry, rewritten to run in linear time, refuses exactly the strings #3838's pattern refused (test/review-risk-redos.test.ts: 60,000 seeded strings and the hand-picked edges, and every SVG blob in the repository's history, with zero differences); SVG_MARKUP now refuses a DOCTYPE naming any document but W3C's SVG DTD (test/review-planner-misreads.test.ts), which only keeps more SVGs code and so drops no reviewer; GST-911's no-loss replay holds unchanged; 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",
|
||||
'GST-898': "GST-898 verdict-identity no-loss check: the linear rm rule, and the rmSync rule (which also raises security on any call with more than 500 characters before its closing parenthesis), give the same verdict as R51's on every changed line of the last 100 main commits (69,023 lines, a563f47 back 100 first-parent commits) and on 60,000 seeded lines per rule (test/review-risk-redos.test.ts), and no commit changes depth, class or risk class, so every diff dispatches main's reviewers unchanged; reviewer text is unchanged",
|
||||
@@ -82,7 +82,7 @@ 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: 'b16f28d6eecc162928d58fcfc6209f703b9692bfe5e901d381972939d2e16f7e', replay: 'GST-911-redos' },
|
||||
'lib/review-depth.ts': { sha: '166dd7b1148a272b345dd9d2ceccd14348cc59c52ef9c88109436f6f61cf6b69', replay: 'GST-911-redos' },
|
||||
'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' },
|
||||
};
|
||||
|
||||
|
||||
@@ -123,7 +123,7 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se
|
||||
}, 60_000);
|
||||
|
||||
// Value: protects=only an SVG made of drawing elements is data: anything that loads, links or declares outside that set stays code;
|
||||
// fails_when=an external image or stylesheet, an entity declaration, an animation or a CSS url() to another document reads as docs,
|
||||
// fails_when=an external image or stylesheet, an entity declaration, an external DTD, an animation or a CSS url() to another document reads as docs,
|
||||
// including through a CSS escape or a zero-padded character reference (review pass 1 findings) or behind a stray quote that
|
||||
// pairs with a later one (review pass 2 finding), or any CSS beyond declarations and url(#id) (Step 11 finding: image-set());
|
||||
// seam=collectCommit + depthOfCommitInput
|
||||
@@ -134,6 +134,8 @@ describe('GST-911 misread 1: a chart SVG is data, an SVG that can run code is se
|
||||
['an animation', ' <animate attributeName="x" from="0" to="10" dur="1s"/>'],
|
||||
['a stylesheet processing instruction', '<?xml-stylesheet href="https://x.test/a.css"?>'],
|
||||
['an entity declaration', '<!DOCTYPE svg [ <!ENTITY e SYSTEM "file:///etc/passwd"> ]>'],
|
||||
['an external DTD', '<!DOCTYPE svg SYSTEM "https://x.test/a.dtd">'],
|
||||
['an external DTD behind the SVG public identifier', '<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "https://x.test/a.dtd">'],
|
||||
['a CSS-escaped import', ' <style>@\\69mport "https://x.test/a.css";</style>'],
|
||||
['a CSS-escaped url()', ' <rect style="fill: u\\72l(https://x.test/a.svg#g)"/>'],
|
||||
['a zero-padded character reference import', ' <style>@import "https://x.test/a.css";</style>'],
|
||||
|
||||
Reference in new issue
Block a user