Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions packages/gittensory-engine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
},
"./miner-goal-spec-parse": {
"types": "./dist/miner-goal-spec-parse.d.ts",
"default": "./dist/miner-goal-spec-parse.js"
},
"./scoring/model": {
"types": "./dist/scoring/model.d.ts",
"default": "./dist/scoring/model.js"
Expand Down
110 changes: 0 additions & 110 deletions packages/gittensory-engine/src/miner-goal-spec-parse.ts

This file was deleted.

10 changes: 5 additions & 5 deletions packages/gittensory-engine/src/miner-goal-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ function normalizeStringList(value: unknown, field: string, warnings: string[]):
}
const result: string[] = [];
const seen = new Set<string>();
for (const entry of value) {
for (const [index, entry] of value.entries()) {
if (index >= MAX_LIST_ITEMS) {
warnings.push(`MinerGoalSpec field "${field}" exceeded ${MAX_LIST_ITEMS} entries; extra entries ignored.`);
break;
}
if (typeof entry !== "string") {
warnings.push(`MinerGoalSpec field "${field}" skipped a non-string entry.`);
continue;
Expand All @@ -118,10 +122,6 @@ function normalizeStringList(value: unknown, field: string, warnings: string[]):
normalized = normalized.slice(0, MAX_ITEM_LENGTH);
}
if (seen.has(normalized)) continue;
if (result.length >= MAX_LIST_ITEMS) {
warnings.push(`MinerGoalSpec field "${field}" exceeded ${MAX_LIST_ITEMS} entries; extra entries ignored.`);
break;
}
result.push(normalized);
seen.add(normalized);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Tests for MinerGoalSpec file discovery (#2294). The tolerant parser itself is covered by
// miner-goal-spec-parse.test.ts / miner-goal-spec-parser.test.ts; this covers only the discovery order. Pure —
// miner-goal-spec-parser.test.ts; this covers only the discovery order. Pure —
// the existence check is injected, so no filesystem is touched. Runs against compiled dist/.
import { test } from "node:test";
import assert from "node:assert/strict";
Expand Down
101 changes: 0 additions & 101 deletions packages/gittensory-engine/test/miner-goal-spec-parse.test.ts

This file was deleted.

51 changes: 51 additions & 0 deletions packages/gittensory-engine/test/miner-goal-spec-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,57 @@ test("parseMinerGoalSpec: exactly 100 unique entries are accepted without a cap
assert.ok(!parsed.warnings.some((warning) => /exceeded 100 entries/i.test(warning)));
});

test("parseMinerGoalSpec: caps inspection of a hostile all-non-string list instead of scanning it in full", () => {
// Regression: the cap used to be checked only after a candidate was accepted (duplicate-check-adjacent), so
// an array of entries that ALWAYS take the `continue` path (non-string, duplicate, or empty-after-trim) never
// hit the cap and got fully scanned -- unbounded CPU/memory work and unbounded warnings for a hostile input.
// The cap must be checked against the raw index, before any per-entry work. `minerEnabled: false` keeps the
// spec "present" regardless of what wantedPaths ends up as, so these assertions stay focused on the list cap.
const parsed = parseMinerGoalSpec({
minerEnabled: false,
wantedPaths: Array.from({ length: 1_000 }, () => null),
});

assert.deepEqual(parsed.spec.wantedPaths, []);
assert.equal(parsed.warnings.length, 101);
assert.match(parsed.warnings.at(-1) ?? "", /exceeded 100 entries/);
});

test("parseMinerGoalSpec: caps inspection of a hostile all-duplicate or all-empty list to a single cap warning", () => {
const duplicates = parseMinerGoalSpec({
minerEnabled: false,
wantedPaths: Array.from({ length: 1_000 }, () => "src/**"),
});
assert.deepEqual(duplicates.spec.wantedPaths, ["src/**"]);
assert.deepEqual(duplicates.warnings, [
'MinerGoalSpec field "wantedPaths" exceeded 100 entries; extra entries ignored.',
]);

const empty = parseMinerGoalSpec({
minerEnabled: false,
wantedPaths: Array.from({ length: 1_000 }, () => " "),
});
assert.deepEqual(empty.spec.wantedPaths, []);
assert.deepEqual(empty.warnings, [
'MinerGoalSpec field "wantedPaths" exceeded 100 entries; extra entries ignored.',
]);
});

test("parseMinerGoalSpec: caps inspection of a hostile all-overlong list, even though each entry still warns once", () => {
// Different from the duplicate/empty case: an overlong entry is TRUNCATED (with its own warning), not
// silently dropped, so every one of the 100 inspected entries produces a truncation warning before the
// post-truncation duplicate check collapses them into one result entry. Still bounded to exactly
// 100 truncate warnings + 1 cap warning, never proportional to the input's true length.
const parsed = parseMinerGoalSpec({
minerEnabled: false,
wantedPaths: Array.from({ length: 1_000 }, () => "x".repeat(300)),
});

assert.deepEqual(parsed.spec.wantedPaths, ["x".repeat(256)]);
assert.equal(parsed.warnings.length, 101);
assert.match(parsed.warnings.at(-1) ?? "", /exceeded 100 entries/);
});

test("parseMinerGoalSpec: malformed fields fall back independently with targeted warnings", () => {
const longEntry = "x".repeat(300);
const parsed = parseMinerGoalSpec({
Expand Down
44 changes: 44 additions & 0 deletions test/unit/miner-goal-spec-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,50 @@ describe("MinerGoalSpec parser (#2301)", () => {
expect(parsed.warnings.join(" ")).not.toMatch(/exceeded 100 entries/i);
});

it("bounds inspection of a hostile all-non-string list instead of scanning it in full", () => {
// Regression: the cap used to be checked only after a candidate was accepted (duplicate-check-adjacent),
// so an array of entries that ALWAYS take the `continue` path (non-string, duplicate, or empty-after-trim)
// never hit the cap and got fully scanned -- unbounded CPU/memory work and unbounded warnings for a hostile
// input. The cap must now be checked against the raw index, before any per-entry work. `minerEnabled: false`
// keeps the spec "present" regardless of what wantedPaths ends up as, so these assertions stay focused on
// the list cap rather than incidentally exercising the separate all-fields-default fallback.
const wantedPaths = Array.from({ length: 1_000 }, () => null);
const parsed = parseMinerGoalSpec({ minerEnabled: false, wantedPaths });

expect(parsed.spec.wantedPaths).toEqual([]);
expect(parsed.warnings).toHaveLength(101);
expect(parsed.warnings.at(-1)).toMatch(/exceeded 100 entries/);
});

it("bounds inspection of a hostile all-duplicate or all-empty list to a single cap warning", () => {
const duplicates = parseMinerGoalSpec({
minerEnabled: false,
wantedPaths: Array.from({ length: 1_000 }, () => "src/**"),
});
expect(duplicates.spec.wantedPaths).toEqual(["src/**"]);
expect(duplicates.warnings).toEqual(['MinerGoalSpec field "wantedPaths" exceeded 100 entries; extra entries ignored.']);

const empty = parseMinerGoalSpec({
minerEnabled: false,
wantedPaths: Array.from({ length: 1_000 }, () => " "),
});
expect(empty.spec.wantedPaths).toEqual([]);
expect(empty.warnings).toEqual(['MinerGoalSpec field "wantedPaths" exceeded 100 entries; extra entries ignored.']);
});

it("bounds inspection of a hostile all-overlong list, even though each inspected entry still warns once", () => {
// Different from the duplicate/empty case: an overlong entry is TRUNCATED (with its own warning), not
// silently dropped, so every one of the 100 inspected entries produces a truncation warning before the
// post-truncation duplicate check collapses them into one result entry. Still bounded to exactly
// 100 truncate warnings + 1 cap warning, never proportional to the input's true length.
const wantedPaths = Array.from({ length: 1_000 }, () => "x".repeat(300));
const parsed = parseMinerGoalSpec({ minerEnabled: false, wantedPaths });

expect(parsed.spec.wantedPaths).toEqual(["x".repeat(256)]);
expect(parsed.warnings).toHaveLength(101);
expect(parsed.warnings.at(-1)).toMatch(/exceeded 100 entries/);
});

it("falls back per field for invalid values without throwing", () => {
const parsed = parseMinerGoalSpec({
minerEnabled: "yes",
Expand Down
Loading