Skip to content

Commit 4fd0441

Browse files
authored
fix: clear the last polynomial-redos instance in swift-cache (#1549)
* fix: clear the last polynomial-redos instance in swift-cache sanitizeCacheName used /^-+|-+$/g to trim edge dashes, the same js/polynomial-redos pattern PR #1546 retired everywhere else. Replace it with the linear-time trim used there, and add a counterfactual regression test that fails against the old regex on a long interior dash run. * fix: keep sanitizeCacheName private, drive redos/fallback pins through compileSwiftSourceText Addresses PR #1549 reviewer feedback: sanitizeCacheName was exported solely so the regression test could import it, which docs/agents/testing.md's test-interface rule forbids. Reverted the export and rewrote the test to exercise the sanitizer through compileSwiftSourceText, an existing production seam that already calls it. - Timing pin: a cache name with a 100k-char interior dash run still resolves in sub-second time (the call may reject once it reaches disk I/O due to the OS path-component length limit, but that happens only after the now-fast sanitize step, so timing the settle either way still proves no catastrophic backtracking). - Fallback pin: a cache name that sanitizes to nothing (e.g. '---') still produces the 'swift-helper' fallback, observed via the returned executable path. Counterfactuals (see PR comment for full output): - Restoring the retired `/^-+|-+$/g` regex trim made the timing pin fail: 3428ms >= 1000ms. - Removing the `|| 'swift-helper'` fallback made the fallback pin fail: basename did not start with 'swift-helper-'.
1 parent 60400d0 commit 4fd0441

2 files changed

Lines changed: 42 additions & 2 deletions

File tree

src/utils/__tests__/swift-cache.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ vi.mock('../exec.ts', () => ({
1313
}));
1414

1515
import { runCmd } from '../exec.ts';
16-
import { compileSwiftSourceFile } from '../swift-cache.ts';
16+
import { compileSwiftSourceFile, compileSwiftSourceText } from '../swift-cache.ts';
1717

1818
const mockRunCmd = vi.mocked(runCmd);
1919

@@ -100,6 +100,33 @@ test('cache lock timeout reports the lock path', async () => {
100100
expect(mockRunCmd).not.toHaveBeenCalled();
101101
});
102102

103+
test('compileSwiftSourceText resolves a cache name with a long interior dash run in sub-second time', async () => {
104+
// Regression pin for the polynomial-regex ReDoS fix in `sanitizeCacheName`'s edge-dash
105+
// trim. The interior dashes are never touched by the trim, so a correct sanitizer never
106+
// needs to inspect this whole run — only a backtracking one pays for its length.
107+
const value = `x${'-'.repeat(100_000)}x`;
108+
109+
const start = Date.now();
110+
// The cache name is long enough to exceed the filesystem's path-component limit, so the
111+
// call is expected to reject once it reaches disk I/O; that happens only *after* the
112+
// (now fast) sanitize step this test pins, so timing the settle either way still proves
113+
// no catastrophic backtracking occurred.
114+
await compileSwiftSourceText({ source: 'print(1)', cacheName: value }).catch(() => {});
115+
const elapsedMs = Date.now() - start;
116+
117+
expect(elapsedMs).toBeLessThan(1_000);
118+
});
119+
120+
test('compileSwiftSourceText falls back to swift-helper when the cache name sanitizes to nothing', async () => {
121+
const executablePath = await compileSwiftSourceText({
122+
source: 'print(1)',
123+
cacheName: '---',
124+
});
125+
126+
expect(path.basename(executablePath).startsWith('swift-helper-')).toBe(true);
127+
expect(fs.statSync(executablePath).mode & 0o111).not.toBe(0);
128+
});
129+
103130
function writeSourceFile(source = 'print("recording")'): string {
104131
const sourcePath = path.join(tmpDir, 'recording-overlay.swift');
105132
fs.writeFileSync(sourcePath, source);

src/utils/swift-cache.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,20 @@ function isExecutableFile(filePath: string): boolean {
169169
}
170170

171171
function sanitizeCacheName(value: string): string {
172-
return value.replaceAll(/[^A-Za-z0-9._-]/g, '-').replaceAll(/^-+|-+$/g, '') || 'swift-helper';
172+
return trimEdgeDashes(value.replaceAll(/[^A-Za-z0-9._-]/g, '-')) || 'swift-helper';
173+
}
174+
175+
/**
176+
* Linear-time edge trim. The regex form (`/^-+|-+$/g`) backtracks
177+
* polynomially on long dash runs (CodeQL js/polynomial-redos), and cache
178+
* names are derived from caller-supplied strings.
179+
*/
180+
function trimEdgeDashes(value: string): string {
181+
let start = 0;
182+
let end = value.length;
183+
while (start < end && value[start] === '-') start += 1;
184+
while (end > start && value[end - 1] === '-') end -= 1;
185+
return value.slice(start, end);
173186
}
174187

175188
function hashParts(parts: Array<string | number | Buffer>): string {

0 commit comments

Comments
 (0)