Skip to content

Commit 30bd532

Browse files
karenchuuYue021130
authored andcommitted
fix(control-plane): make the digest ownership scan see constructed matchers too
Signed-off-by: karenchuu <25980598+karenchuu@users.noreply.github.com>
1 parent 57a1859 commit 30bd532

1 file changed

Lines changed: 73 additions & 37 deletions

File tree

‎tests/control_plane_ts/content_digest_single_owner.test.ts‎

Lines changed: 73 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,18 @@ import { decodeOutboxCursor } from "../../loopx/control_plane/coordination/local
1414

1515
const PACKAGE_ROOT = new URL("../../loopx", import.meta.url).pathname;
1616
const OWNER_FILE = "control_plane/content_digest.ts";
17-
const WHOLE_VALUE = /\/\^(?:sha256:)?\[[0-9a-f-]{6}\]\{64\}\$\/([a-z]*)/g;
18-
const HEX_CLASSES = new Set(["0-9a-f", "a-f0-9"]);
1917

20-
/** Sites that keep their own literal, each with the reason a reviewer can check. */
18+
// The same decision is written two ways: a regex literal, and a constructed RegExp over
19+
// an equivalent string. Both are forbidden outside the owner, and the bypass cases below
20+
// are what stop this scan from quietly decaying into a search for one spelling.
21+
const WHOLE_VALUE = /\/\^(?:sha256:)?\[[0-9a-f-]{6}\]\{64\}\$\/(?<flags>[a-z]*)/g;
22+
const CONSTRUCTED = /(?:new\s+)?RegExp\(\s*(?:["'`])(?<body>\^?(?:sha256:)?\[[0-9a-f-]{6}\]\{64\}\$?)(?:["'`])/g;
23+
24+
/** Sites that keep their own matcher, each with a reason a reviewer can check. */
2125
const RECORDED_EXCEPTIONS: Record<string, string> = {
2226
"control_plane/agents/delivery_workspace.ts":
23-
"GIT_REVISION_DIGEST_PATTERN is case-insensitive (/i): a git revision may be " +
24-
"written in either case, so this surface is a different policy, not a stale copy.",
27+
"GIT_REVISION_DIGEST_PATTERN is case-insensitive (/i): a git revision may be written " +
28+
"in either case, so this surface is a different policy, not a stale copy.",
2529
};
2630

2731
function tsFiles(dir: string, base = ""): string[] {
@@ -34,50 +38,81 @@ function tsFiles(dir: string, base = ""): string[] {
3438
return found;
3539
}
3640

37-
function wholeValueSites(): { file: string; line: number; literal: string; flags: string }[] {
38-
const rows: { file: string; line: number; literal: string; flags: string }[] = [];
41+
type Site = { file?: string; line: number; literal: string; flags: string };
42+
43+
function sitesInText(text: string): Site[] {
44+
const rows: Site[] = [];
45+
text.split("\n").forEach((line, index) => {
46+
for (const match of line.matchAll(WHOLE_VALUE)) {
47+
rows.push({ line: index + 1, literal: match[0], flags: match.groups?.flags ?? "" });
48+
}
49+
for (const match of line.matchAll(CONSTRUCTED)) {
50+
rows.push({ line: index + 1, literal: `new RegExp(${match.groups?.body})`, flags: "" });
51+
}
52+
});
53+
return rows;
54+
}
55+
56+
function wholeValueSites(): Site[] {
57+
const rows: Site[] = [];
3958
for (const file of tsFiles(PACKAGE_ROOT)) {
4059
if (file.endsWith(".generated.ts")) continue;
41-
const text = readFileSync(join(PACKAGE_ROOT, file), "utf8");
42-
const lines = text.split("\n");
43-
lines.forEach((line, index) => {
44-
for (const match of line.matchAll(WHOLE_VALUE)) {
45-
const body = match[0].slice(1, match[0].length - 1 - (match[1] ?? "").length);
46-
const classMatch = /\[([0-9a-f-]{6})\]/.exec(body);
47-
if (!classMatch || !HEX_CLASSES.has(classMatch[1])) continue;
48-
rows.push({ file, line: index + 1, literal: match[0], flags: match[1] ?? "" });
49-
}
50-
});
60+
for (const hit of sitesInText(readFileSync(join(PACKAGE_ROOT, file), "utf8"))) {
61+
rows.push({ ...hit, file });
62+
}
5163
}
5264
return rows;
5365
}
5466

5567
test("the whole-value digest shape is stated once, outside recorded exceptions", () => {
5668
const offenders = wholeValueSites().filter(
57-
(row) => row.file !== OWNER_FILE && !(row.file in RECORDED_EXCEPTIONS),
69+
(row) => row.file !== OWNER_FILE && !(row.file! in RECORDED_EXCEPTIONS),
5870
);
5971
assert.deepEqual(
6072
offenders.map((row) => `${row.file}:${row.line} ${row.literal}`),
6173
[],
6274
);
6375
});
6476

77+
test("a constructed private matcher is the same violation as a literal one", () => {
78+
// The bypass that made the first version of this guard unable to keep its promise:
79+
// a consumer re-derives the shape with `new RegExp`, behaviour is unchanged, and a
80+
// literal-only scan sees nothing.
81+
const constructed =
82+
'export function check(value: string): boolean {\n' +
83+
' return new RegExp("^[0-9a-f]{64}$").test(value);\n' +
84+
'}\n';
85+
assert.equal(sitesInText(constructed).length, 1, "new RegExp bypass escaped the scan");
86+
87+
const literal =
88+
"export function check(value: string): boolean {\n" +
89+
" return /^[0-9a-f]{64}$/.test(value);\n" +
90+
"}\n";
91+
assert.equal(sitesInText(literal).length, 1, "literal restatement escaped the scan");
92+
93+
const templated =
94+
"export const CHECK = (v: string) => new RegExp(`^sha256:[0-9a-f]{64}$`).test(v);\n";
95+
assert.equal(sitesInText(templated).length, 1, "template-literal bypass escaped the scan");
96+
97+
// A case-insensitive git revision rule is a different policy and stays out of scope
98+
// for the ownership assertion; it is the one recorded exception.
99+
const caseInsensitive = 'const REVISION = /^[0-9a-f]{64}$/i;\n';
100+
assert.equal(sitesInText(caseInsensitive)[0]?.flags, "i");
101+
});
102+
65103
test("the owner module states each envelope exactly once", () => {
66-
const owner = wholeValueSites().filter((row) => row.file === OWNER_FILE);
67-
assert.equal(owner.length, 2);
68-
assert.equal(
69-
owner.filter((row) => row.literal.includes("sha256:")).length,
70-
1,
71-
);
72-
assert.deepEqual([...new Set(owner.map((row) => row.flags))], [""]);
104+
const sites = sitesInText(readFileSync(join(PACKAGE_ROOT, OWNER_FILE), "utf8"));
105+
assert.equal(sites.length, 2, JSON.stringify(sites));
106+
assert.equal(sites.filter((row) => row.literal.includes("sha256:")).length, 1);
107+
assert.deepEqual([...new Set(sites.map((row) => row.flags))], [""]);
73108
});
74109

75110
test("a recorded exception is still the reason it was recorded", () => {
76111
for (const file of Object.keys(RECORDED_EXCEPTIONS)) {
77-
const rows = wholeValueSites().filter((row) => row.file === file);
78-
assert.ok(rows.length > 0, `${file} no longer restates the shape; drop the exception`);
112+
const sites = sitesInText(readFileSync(join(PACKAGE_ROOT, file), "utf8"));
113+
assert.ok(sites.length > 0, `${file} no longer restates the shape; drop the exception`);
79114
assert.ok(
80-
rows.every((row) => row.flags.length > 0),
115+
sites.every((row) => row.flags.length > 0),
81116
`${file} lost its per-surface flags; absorb it into the owner instead`,
82117
);
83118
}
@@ -86,16 +121,17 @@ test("a recorded exception is still the reason it was recorded", () => {
86121
test("class order cannot change a verdict", () => {
87122
const first = /^[a-f0-9]{64}$/;
88123
const second = /^[0-9a-f]{64}$/;
89-
for (const probe of ["b".repeat(64), "0123456789abcdef".repeat(4), "B".repeat(64), "b".repeat(63), "z".repeat(64)]) {
124+
for (const probe of ["b".repeat(64), "0123456789abcdef".repeat(4), "B".repeat(64), "b".repeat(63), "g".repeat(64)]) {
90125
assert.equal(first.test(probe), second.test(probe), probe);
91126
assert.equal(second.test(probe), BARE_SHA256_PATTERN.test(probe), probe);
92127
}
93128
});
94129

95130
test("dropping the unicode flag cannot change a verdict for this pattern", () => {
96131
const flagged = /^[a-f0-9]{64}$/u;
97-
const probes = ["b".repeat(64), "sha256:" + "b".repeat(64), "𝟏".repeat(64), "b".repeat(64) + "𝟏"];
98-
for (const probe of probes) assert.equal(flagged.test(probe), BARE_SHA256_PATTERN.test(probe), probe);
132+
for (const probe of ["b".repeat(64), "sha256:" + "b".repeat(64), "\u{1D7CF}".repeat(64)]) {
133+
assert.equal(flagged.test(probe), BARE_SHA256_PATTERN.test(probe), probe);
134+
}
99135
});
100136

101137
test("the bare envelope rejects the prefixed form and vice versa", () => {
@@ -118,10 +154,7 @@ test("promotion plan digest is read through the owner as a bare digest", () => {
118154

119155
test("delegation inventory cursor is a bare digest, not an enveloped one", () => {
120156
const hex = "c".repeat(64);
121-
assert.deepEqual(
122-
delegationInventoryQuery({ limit: 5, cursor: hex }).cursor,
123-
hex,
124-
);
157+
assert.equal(delegationInventoryQuery({ limit: 5, cursor: hex }).cursor, hex);
125158
assert.throws(
126159
() => delegationInventoryQuery({ limit: 5, cursor: `sha256:${hex}` }),
127160
/invalid delegation inventory cursor/,
@@ -158,7 +191,10 @@ test("drain cursor keeps its envelope and its unbound option", () => {
158191
last_provider_revision: "r1",
159192
updated_at: "2026-09-28T00:00:00Z",
160193
});
161-
assert.equal(decodeOutboxCursor(cursor(`sha256:${hex}`), "todos").last_partition_digest, `sha256:${hex}`);
194+
assert.equal(
195+
decodeOutboxCursor(cursor(`sha256:${hex}`), "todos").last_partition_digest,
196+
`sha256:${hex}`,
197+
);
162198
assert.equal(decodeOutboxCursor(cursor(null), "todos").last_partition_digest, null);
163-
assert.throws(() => decodeOutboxCursor(cursor(hex), "todos"), /drain cursor binding is invalid/);
199+
assert.throws(() => decodeOutboxCursor(cursor(hex), "todos"), /drain cursor binding/);
164200
});

0 commit comments

Comments
 (0)