Skip to content

Commit 4f078fc

Browse files
committed
test(metadata-core): make the ReDoS guard load-insensitive (#4485)
The ReDoS assertion in protocol-handshake.test.ts bounded the pathological scan with an absolute 50ms wall clock. Under the full-repo run (~130 parallel turbo tasks) that ceiling measures machine load rather than the parser: it exceeded 50ms on a healthy tree and reddened PRs that never touched this package, leaving the diagnosis cost to whoever happened to be running. The underlying guard is real and stays (CodeQL 837/838). What changes is how it is measured. The three `toBeNull()` behavioural assertions -- the actual contract, that adversarial input is unrecognized rather than falsely rejected -- are kept and now stand on their own. The wall-clock proxy is replaced by a scaling check: the same adversarial shapes at 1x and 8x length, asserting the parse stays linear in the input. Load largely cancels out of a ratio, which is what makes the criterion load-insensitive. Measured: healthy parsing tracks the input at 8.3-8.5x, stable across runs. The 40x ceiling keeps ~5x headroom while still catching a merely quadratic regression (~64x), let alone an exponential one, which would not finish. Two measurement details are load-hardening, both established empirically: timings are taken back-to-back within one iteration and reduced by minimum *ratio* rather than minimising each timing independently (a scheduler steal landing in only one window skewed the latter, observed reddening at 3x CPU oversubscription); and the JIT is warmed so the baseline is not inflated. Note the pathological-to-benign ratio suggested on the issue does not work here: a benign 16-char range parses ~300x faster than a 100k-char one purely because it is 100k characters shorter, so it would fail on a healthy machine. Verification: 5/5 consecutive clean runs, plus 8/8 under 3x CPU oversubscription (the shape that reproduced the original failure). Fixes #4485
1 parent 0f9faa2 commit 4f078fc

1 file changed

Lines changed: 57 additions & 8 deletions

File tree

packages/metadata-core/src/protocol-handshake.test.ts

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,65 @@ describe('rangeAdmitsMajor', () => {
6464
expect(rangeAdmitsMajor('workspace:*', 11)).toBeNull();
6565
});
6666

67-
it('bounds pathological input (ReDoS-safe) without a slow scan', () => {
67+
it('bounds pathological input (ReDoS-safe) without catastrophic backtracking', () => {
6868
// The engines string is externally authored; the comparator/hyphen parsing
6969
// must not degrade on adversarial input (CodeQL alerts 837/838).
70-
const overlong = '<' + '\t'.repeat(100_000);
71-
const hyphenBomb = 'a\t-\t' + '\t'.repeat(100_000);
72-
const start = performance.now();
73-
expect(rangeAdmitsMajor(overlong, 11)).toBeNull();
74-
expect(rangeAdmitsMajor(hyphenBomb, 11)).toBeNull();
75-
expect(rangeAdmitsMajor('>=11.0.0 ' + ' '.repeat(100_000) + '<13.0.0', 11)).toBeNull();
76-
expect(performance.now() - start).toBeLessThan(50);
70+
//
71+
// The adversarial shapes: an overlong comparator, a hyphen-range "bomb", and
72+
// a comparator pair separated by a huge whitespace run.
73+
const shapes = (scale: number) => [
74+
'<' + '\t'.repeat(scale),
75+
'a\t-\t' + '\t'.repeat(scale),
76+
'>=11.0.0 ' + ' '.repeat(scale) + '<13.0.0',
77+
];
78+
79+
// 1. Behaviour: every shape is *unrecognized*, never a false rejection.
80+
// This is the assertion that actually pins the contract.
81+
for (const input of shapes(100_000)) {
82+
expect(rangeAdmitsMajor(input, 11)).toBeNull();
83+
}
84+
85+
// 2. Cost: the parse must stay linear in the input length.
86+
//
87+
// This deliberately asserts *no absolute wall-clock bound*. The previous
88+
// 50ms ceiling measured machine load rather than the parser: under the
89+
// full-repo run (~130 parallel turbo tasks) it exceeded 50ms on a healthy
90+
// tree and reddened PRs that never touched this package (#4485).
91+
//
92+
// What the guard is really for is catastrophic backtracking — a regression
93+
// that makes parsing *superlinear* in the input. So measure the scaling
94+
// instead: the same shapes at 1x and 8x length. Load largely cancels out of
95+
// a ratio, and min-of-N discards the samples the scheduler interrupted.
96+
//
97+
// Healthy (linear) parsing tracks the input at ~8x; measured repeatedly at
98+
// 8.3-8.5x. The 40x ceiling therefore keeps ~5x headroom over healthy while
99+
// still catching even a merely *quadratic* regression (which lands near
100+
// 64x), let alone an exponential one — which would not finish at all.
101+
//
102+
// The two timings are taken back-to-back inside one iteration and the ratio
103+
// is reduced by *minimum*, not the timings independently: a scheduler steal
104+
// that lands in only one of the two windows would skew a ratio built from
105+
// separately-minimised timings (observed reddening at 3x CPU
106+
// oversubscription), whereas the cheapest single pair is the one iteration
107+
// that ran cleanest end to end.
108+
//
109+
// NB: a ratio of pathological-to-benign input would NOT work here: a benign
110+
// 16-char range parses ~300x faster than a 100k-char one purely because it
111+
// is 100k characters shorter, which is linear scaling behaving correctly.
112+
const small = shapes(100_000);
113+
const big = shapes(800_000);
114+
const scan = (inputs: readonly string[]): number => {
115+
const t = performance.now();
116+
for (const input of inputs) rangeAdmitsMajor(input, 11);
117+
return performance.now() - t;
118+
};
119+
120+
for (let i = 0; i < 10; i++) scan(small); // warm the JIT before measuring
121+
let ratio = Infinity;
122+
for (let i = 0; i < 20; i++) {
123+
ratio = Math.min(ratio, scan(big) / scan(small));
124+
}
125+
expect(ratio).toBeLessThan(40);
77126
});
78127
});
79128

0 commit comments

Comments
 (0)