Skip to content

Commit 46f62fd

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(design): refuse unsafe generated output paths
1 parent 24ba7a2 commit 46f62fd

2 files changed

Lines changed: 190 additions & 12 deletions

File tree

‎scripts/generate-design-contracts.test.ts‎

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
import { spawnSync } from 'node:child_process'
2-
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
2+
import {
3+
existsSync,
4+
mkdirSync,
5+
mkdtempSync,
6+
readdirSync,
7+
readFileSync,
8+
rmSync,
9+
symlinkSync,
10+
writeFileSync,
11+
} from 'node:fs'
312
import { tmpdir } from 'node:os'
413
import path from 'node:path'
514
import { afterEach, expect, test } from 'vitest'
@@ -58,6 +67,8 @@ afterEach(() => {
5867

5968
test('CLI discovers new APIs, narrows recipe variants to public props and is byte deterministic', () => {
6069
const root = fixture()
70+
expect(run(root, '--check').status).toBe(1)
71+
expect(existsSync(path.join(root, 'scripts'))).toBe(false)
6172
const first = run(root)
6273
expect(first.status, first.stderr).toBe(0)
6374
const bytes = readFileSync(path.join(root, artifact), 'utf8')
@@ -73,6 +84,112 @@ test('CLI discovers new APIs, narrows recipe variants to public props and is byt
7384
expect(readFileSync(path.join(root, artifact), 'utf8')).toBe('{}')
7485
}, 60_000)
7586

87+
test.each(['file', 'dangling', 'scripts', 'design-conformance'])(
88+
'generation rejects a linked %s output boundary without modifying external files',
89+
(kind) => {
90+
const root = fixture()
91+
const external = mkdtempSync(path.join(tmpdir(), 'design-output-target-'))
92+
roots.push(external)
93+
const sentinel = path.join(external, 'sentinel')
94+
writeFileSync(sentinel, 'external bytes must survive')
95+
if (kind === 'file' || kind === 'dangling') {
96+
mkdirSync(path.dirname(path.join(root, artifact)), { recursive: true })
97+
symlinkSync(
98+
kind === 'file' ? sentinel : path.join(external, 'missing'),
99+
path.join(root, artifact)
100+
)
101+
} else {
102+
const parent = kind === 'scripts' ? 'scripts' : path.dirname(artifact)
103+
mkdirSync(path.dirname(path.join(root, parent)), { recursive: true })
104+
symlinkSync(external, path.join(root, parent), 'dir')
105+
}
106+
const child = run(root)
107+
expect(readFileSync(sentinel, 'utf8')).toBe('external bytes must survive')
108+
expect(readdirSync(external)).toEqual(['sentinel'])
109+
expect(child.status, child.stderr).toBe(2)
110+
expect(run(root, '--check').status).toBe(2)
111+
expect(readdirSync(external)).toEqual(['sentinel'])
112+
},
113+
60_000
114+
)
115+
116+
test.each(['symlink', 'fifo', 'nonregular', 'invalid-descriptor'])(
117+
'generation refuses an output changed to %s at descriptor opening',
118+
(kind) => {
119+
const root = fixture()
120+
const external = mkdtempSync(path.join(tmpdir(), 'design-output-race-'))
121+
roots.push(external)
122+
const sentinel = path.join(external, 'sentinel')
123+
writeFileSync(sentinel, 'external bytes must survive')
124+
write(root, artifact, 'existing output must survive validation failure')
125+
const output = path.join(root, artifact)
126+
const preload = path.join(external, 'replace-output.cjs')
127+
const attack =
128+
kind === 'symlink'
129+
? `fs.unlinkSync(output);fs.symlinkSync(${JSON.stringify(sentinel)},output);`
130+
: kind === 'invalid-descriptor'
131+
? 'const descriptor=open(file,flags,mode);fs.closeSync(descriptor);return descriptor;'
132+
: `fs.unlinkSync(output);require('node:child_process').execFileSync('mkfifo',[output]);${kind === 'nonregular' ? 'open(output,fs.constants.O_RDWR|fs.constants.O_NONBLOCK);' : ''}`
133+
writeFileSync(
134+
preload,
135+
`
136+
const fs=require('node:fs');const requested=${JSON.stringify(output)};const output=fs.realpathSync(requested);const open=fs.openSync;const write=fs.writeFileSync;
137+
fs.openSync=(file,flags,mode)=>{if((file===output||file===requested)&&(flags&fs.constants.O_WRONLY)){process.stderr.write('Output replacement applied\\n');${attack}}return open(file,flags,mode)};
138+
fs.writeFileSync=(file,...args)=>{if(file===output||file===requested){const descriptor=fs.openSync(file,fs.constants.O_WRONLY|fs.constants.O_CREAT|fs.constants.O_TRUNC,0o644);try{return write(descriptor,...args)}finally{fs.closeSync(descriptor)}}return write(file,...args)};
139+
`
140+
)
141+
const child = spawnSync(
142+
bun,
143+
['--no-env-file', '--preload', preload, cli, '--repo', root, '--ref', 'HEAD'],
144+
{
145+
encoding: 'utf8',
146+
timeout: 5000,
147+
}
148+
)
149+
expect(child.stderr).toContain('Output replacement applied')
150+
expect(child.status, child.stderr).toBe(2)
151+
expect(readFileSync(sentinel, 'utf8')).toBe('external bytes must survive')
152+
if (kind === 'invalid-descriptor')
153+
expect(readFileSync(output, 'utf8')).toBe('existing output must survive validation failure')
154+
if (kind === 'nonregular') expect(child.stderr).toContain('not a regular file')
155+
},
156+
15_000
157+
)
158+
159+
test('generation rejects a directory output and preserves immutable checks through a repository alias', () => {
160+
const root = fixture()
161+
mkdirSync(path.join(root, artifact), { recursive: true })
162+
write(root, `${artifact}/sentinel`, 'directory bytes must survive')
163+
expect(run(root, '--ref', 'HEAD').status).toBe(2)
164+
expect(readFileSync(path.join(root, artifact, 'sentinel'), 'utf8')).toBe(
165+
'directory bytes must survive'
166+
)
167+
rmSync(path.join(root, artifact), { recursive: true })
168+
expect(run(root).status).toBe(0)
169+
git(root, 'add', '.')
170+
git(
171+
root,
172+
'-c',
173+
'user.name=Test',
174+
'-c',
175+
'user.email=test@example.com',
176+
'commit',
177+
'-qm',
178+
'generated'
179+
)
180+
const external = mkdtempSync(path.join(tmpdir(), 'design-output-alias-'))
181+
roots.push(external)
182+
const alias = path.join(external, 'repo')
183+
symlinkSync(root, alias, 'dir')
184+
expect(run(alias).status).toBe(0)
185+
const sentinel = path.join(external, 'sentinel')
186+
writeFileSync(sentinel, 'external bytes must survive')
187+
rmSync(path.join(root, artifact))
188+
symlinkSync(sentinel, path.join(root, artifact))
189+
expect(run(alias, '--check', '--ref', 'HEAD').status).toBe(0)
190+
expect(readFileSync(sentinel, 'utf8')).toBe('external bytes must survive')
191+
}, 60_000)
192+
76193
test('nested finite style lookups own only the selected slot chrome', () => {
77194
const root = fixture()
78195
write(

‎scripts/generate-design-contracts.ts‎

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,15 @@
1-
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'
1+
import {
2+
closeSync,
3+
constants,
4+
fstatSync,
5+
ftruncateSync,
6+
lstatSync,
7+
mkdirSync,
8+
openSync,
9+
readFileSync,
10+
realpathSync,
11+
writeFileSync,
12+
} from 'node:fs'
213
import path from 'node:path'
314
import { parseArgs } from 'node:util'
415
import {
@@ -8,6 +19,29 @@ import {
819
} from '#design-conformance/generated-contracts'
920
import { GitSource } from '#design-conformance/worktree-source'
1021

22+
/** Validate each parent before creating descendants; linked parents can redirect missing outputs. */
23+
function outputParents(repo: string, create = false): string {
24+
let parent = repo
25+
for (const part of path.dirname(GENERATED_FILE).split('/')) {
26+
parent = path.join(parent, part)
27+
let stat = lstatSync(parent, { throwIfNoEntry: false })
28+
if (!stat && create) {
29+
mkdirSync(parent)
30+
stat = lstatSync(parent)
31+
}
32+
if (stat && !stat.isDirectory())
33+
throw new Error(`Generated output has an unsafe parent directory: ${parent}`)
34+
}
35+
return path.join(repo, GENERATED_FILE)
36+
}
37+
38+
/** Preflight before working-tree inventory so unsafe outputs are never read as source inputs. */
39+
function checkedOutput(repo: string): void {
40+
const output = outputParents(repo)
41+
const stat = lstatSync(output, { throwIfNoEntry: false })
42+
if (stat && !stat.isFile()) throw new Error('Generated output is not a regular file')
43+
}
44+
1145
export async function main(argv = process.argv.slice(2)): Promise<number> {
1246
try {
1347
const { values } = parseArgs({
@@ -20,30 +54,57 @@ export async function main(argv = process.argv.slice(2)): Promise<number> {
2054
},
2155
})
2256
const start = performance.now()
23-
const source = new GitSource(values.repo, values.ref ?? 'HEAD', !values.ref)
57+
const repo = realpathSync(values.repo)
58+
if (!values.check || !values.ref) checkedOutput(repo)
59+
const source = new GitSource(repo, values.ref ?? 'HEAD', !values.ref)
2460
const generated = await generateContracts(source.central())
2561
source.assertUnchanged()
26-
const output = path.join(values.repo, GENERATED_FILE)
62+
const output = path.join(source.repo, GENERATED_FILE)
2763
const bytes = generatedBytes(generated)
2864
if (values.check) {
2965
let actual: string | undefined
3066
try {
3167
const entry = source.entries.find((e) => e.path === GENERATED_FILE)
32-
actual = values.ref
33-
? entry
34-
? source.read(entry)
35-
: undefined
36-
: readFileSync(output, 'utf8')
37-
} catch {}
68+
if (values.ref) actual = entry ? source.read(entry) : undefined
69+
else {
70+
const descriptor = openSync(
71+
output,
72+
constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK
73+
)
74+
try {
75+
if (!fstatSync(descriptor).isFile())
76+
throw new Error('Generated output is not a regular file')
77+
outputParents(source.repo)
78+
actual = readFileSync(descriptor, 'utf8')
79+
} finally {
80+
closeSync(descriptor)
81+
}
82+
}
83+
} catch {
84+
actual = undefined
85+
}
3886
if (actual !== bytes) {
3987
process.stderr.write(
4088
'Design infrastructure is missing, malformed or stale. Run bun run design:generate and commit contracts.generated.json.\n'
4189
)
4290
return 1
4391
}
4492
} else {
45-
mkdirSync(path.dirname(output), { recursive: true })
46-
writeFileSync(output, bytes)
93+
outputParents(source.repo, true)
94+
const descriptor = openSync(
95+
output,
96+
constants.O_WRONLY | constants.O_CREAT | constants.O_NOFOLLOW | constants.O_NONBLOCK,
97+
0o644
98+
)
99+
try {
100+
if (!fstatSync(descriptor).isFile())
101+
throw new Error('Generated output is not a regular file')
102+
outputParents(source.repo)
103+
ftruncateSync(descriptor, 0)
104+
writeFileSync(descriptor, bytes)
105+
} finally {
106+
closeSync(descriptor)
107+
}
47108
}
48109
process.stdout.write(
49110
`${values.check ? 'Verified' : 'Generated'} ${Object.keys(generated.exports).length} exports, ${Object.keys(generated.recipes).length} recipes, ${Object.keys(generated.tokens).length} tokens, ${generated.diagnostics.length} diagnostics; ${Buffer.byteLength(bytes)} bytes; ${Math.round(performance.now() - start)}ms; ${process.resourceUsage().maxRSS} KiB max RSS\n`

0 commit comments

Comments
 (0)