Skip to content

Commit 63e4b7f

Browse files
kenryu42kenryu42
authored
Merge pull request #31 from kenryu42/fix/tmpdir-traversal-bypass
Fix: tmpdir traversal bypass
2 parents 82f676b + 3e7df7f commit 63e4b7f

14 files changed

Lines changed: 208 additions & 90 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,42 @@ on:
55
push:
66
branches: [main]
77
paths:
8+
- ".github/workflows/**"
89
- "src/**"
910
- "scripts/**"
1011
- "tests/**"
12+
- "assets/**"
13+
- "package.json"
14+
- "bun.lock"
15+
- "tsconfig.json"
16+
- "tsconfig.build.json"
17+
- "biome.json"
18+
- "knip.ts"
19+
- "sgconfig.yml"
20+
- "ast-grep/**"
1121
pull_request:
1222
branches: [main]
1323
paths:
24+
- ".github/workflows/**"
1425
- "src/**"
1526
- "scripts/**"
1627
- "tests/**"
28+
- "assets/**"
29+
- "package.json"
30+
- "bun.lock"
31+
- "tsconfig.json"
32+
- "tsconfig.build.json"
33+
- "biome.json"
34+
- "knip.ts"
35+
- "sgconfig.yml"
36+
- "ast-grep/**"
1737

1838
concurrency:
1939
group: ${{ github.workflow }}-${{ github.ref }}
2040
cancel-in-progress: true
2141

2242
jobs:
23-
quality:
43+
full-check:
2444
runs-on: ubuntu-latest
2545

2646
steps:
@@ -31,39 +51,12 @@ jobs:
3151
bun-version: latest
3252

3353
- name: Install dependencies
34-
run: bun install
54+
run: bun install --frozen-lockfile
3555
env:
3656
BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/cli"
3757

38-
- name: Run linter
39-
run: bun run lint
40-
41-
- name: Run type checker
42-
run: bun run typecheck
43-
44-
- name: Run dead code detection
45-
run: bun run knip
46-
47-
- name: Run ast-grep scan
48-
run: bun run sg:scan
49-
50-
test:
51-
runs-on: ubuntu-latest
52-
53-
steps:
54-
- uses: actions/checkout@v4
55-
56-
- uses: oven-sh/setup-bun@v2
57-
with:
58-
bun-version: latest
59-
60-
- name: Install dependencies
61-
run: bun install
62-
env:
63-
BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/cli"
64-
65-
- name: Run tests with coverage
66-
run: AGENT=1 bun test --coverage --coverage-reporter=lcov
58+
- name: Run full checks
59+
run: bun run check:ci
6760

6861
- name: Upload coverage reports to Codecov
6962
uses: codecov/codecov-action@v5
@@ -72,7 +65,7 @@ jobs:
7265

7366
build:
7467
runs-on: ubuntu-latest
75-
needs: [quality, test]
68+
needs: [full-check]
7669
permissions:
7770
contents: write
7871

@@ -86,7 +79,7 @@ jobs:
8679
bun-version: latest
8780

8881
- name: Install dependencies
89-
run: bun install
82+
run: bun install --frozen-lockfile
9083
env:
9184
BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/cli"
9285

‎.github/workflows/publish.yml‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ permissions:
2424
id-token: write
2525

2626
jobs:
27-
test:
27+
full-check:
2828
runs-on: ubuntu-latest
2929
steps:
3030
- uses: actions/checkout@v4
@@ -34,33 +34,16 @@ jobs:
3434
bun-version: latest
3535

3636
- name: Install dependencies
37-
run: bun install
38-
env:
39-
BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/cli"
40-
41-
- name: Run tests
42-
run: AGENT=1 bun test --coverage
43-
44-
typecheck:
45-
runs-on: ubuntu-latest
46-
steps:
47-
- uses: actions/checkout@v4
48-
49-
- uses: oven-sh/setup-bun@v2
50-
with:
51-
bun-version: latest
52-
53-
- name: Install dependencies
54-
run: bun install
37+
run: bun install --frozen-lockfile
5538
env:
5639
BUN_INSTALL_ALLOW_SCRIPTS: "@ast-grep/cli"
5740

58-
- name: Type check
59-
run: bun run typecheck
41+
- name: Run full checks
42+
run: bun run check:ci
6043

6144
publish:
6245
runs-on: ubuntu-latest
63-
needs: [test, typecheck]
46+
needs: [full-check]
6447
if: github.repository == 'kenryu42/claude-code-safety-net'
6548
steps:
6649
- uses: actions/checkout@v4

‎dist/bin/cc-safety-net.js‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,10 +1580,6 @@ function splitShellCommands(command) {
15801580
let i = 0;
15811581
while (i < tokens.length) {
15821582
const token = tokens[i];
1583-
if (token === undefined) {
1584-
i++;
1585-
continue;
1586-
}
15871583
if (isOperator(token)) {
15881584
if (current.length > 0) {
15891585
segments.push(current);
@@ -1714,9 +1710,6 @@ function parseEnvAssignment(token) {
17141710
return null;
17151711
}
17161712
const eqIdx = token.indexOf("=");
1717-
if (eqIdx < 0) {
1718-
return null;
1719-
}
17201713
return { name: token.slice(0, eqIdx), value: token.slice(eqIdx + 1) };
17211714
}
17221715
function stripEnvAssignmentsWithInfo(tokens) {
@@ -2736,6 +2729,7 @@ function parseParallelCommand(tokens) {
27362729

27372730
// src/core/analyze/tmpdir.ts
27382731
import { tmpdir as tmpdir2 } from "node:os";
2732+
import { normalize as normalize2, sep as sep2 } from "node:path";
27392733
function isTmpdirOverriddenToNonTemp(envAssignments) {
27402734
if (!envAssignments.has("TMPDIR")) {
27412735
return false;
@@ -2744,8 +2738,9 @@ function isTmpdirOverriddenToNonTemp(envAssignments) {
27442738
if (tmpdirValue === "") {
27452739
return true;
27462740
}
2747-
const sysTmpdir = tmpdir2();
2748-
if (isPathOrSubpath(tmpdirValue, "/tmp") || isPathOrSubpath(tmpdirValue, "/var/tmp") || isPathOrSubpath(tmpdirValue, sysTmpdir)) {
2741+
const normalizedTmpdirValue = normalize2(tmpdirValue);
2742+
const sysTmpdir = normalize2(tmpdir2());
2743+
if (isPathOrSubpath(normalizedTmpdirValue, normalize2("/tmp")) || isPathOrSubpath(normalizedTmpdirValue, normalize2("/var/tmp")) || isPathOrSubpath(normalizedTmpdirValue, sysTmpdir)) {
27492744
return false;
27502745
}
27512746
return true;
@@ -2754,7 +2749,7 @@ function isPathOrSubpath(path, basePath) {
27542749
if (path === basePath) {
27552750
return true;
27562751
}
2757-
const baseWithSlash = basePath.endsWith("/") ? basePath : `${basePath}/`;
2752+
const baseWithSlash = basePath.endsWith(sep2) ? basePath : `${basePath}${sep2}`;
27582753
return path.startsWith(baseWithSlash);
27592754
}
27602755

‎dist/index.js‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -441,10 +441,6 @@ function splitShellCommands(command) {
441441
let i = 0;
442442
while (i < tokens.length) {
443443
const token = tokens[i];
444-
if (token === undefined) {
445-
i++;
446-
continue;
447-
}
448444
if (isOperator(token)) {
449445
if (current.length > 0) {
450446
segments.push(current);
@@ -575,9 +571,6 @@ function parseEnvAssignment(token) {
575571
return null;
576572
}
577573
const eqIdx = token.indexOf("=");
578-
if (eqIdx < 0) {
579-
return null;
580-
}
581574
return { name: token.slice(0, eqIdx), value: token.slice(eqIdx + 1) };
582575
}
583576
function stripEnvAssignmentsWithInfo(tokens) {
@@ -1597,6 +1590,7 @@ function parseParallelCommand(tokens) {
15971590

15981591
// src/core/analyze/tmpdir.ts
15991592
import { tmpdir as tmpdir2 } from "node:os";
1593+
import { normalize as normalize2, sep as sep2 } from "node:path";
16001594
function isTmpdirOverriddenToNonTemp(envAssignments) {
16011595
if (!envAssignments.has("TMPDIR")) {
16021596
return false;
@@ -1605,8 +1599,9 @@ function isTmpdirOverriddenToNonTemp(envAssignments) {
16051599
if (tmpdirValue === "") {
16061600
return true;
16071601
}
1608-
const sysTmpdir = tmpdir2();
1609-
if (isPathOrSubpath(tmpdirValue, "/tmp") || isPathOrSubpath(tmpdirValue, "/var/tmp") || isPathOrSubpath(tmpdirValue, sysTmpdir)) {
1602+
const normalizedTmpdirValue = normalize2(tmpdirValue);
1603+
const sysTmpdir = normalize2(tmpdir2());
1604+
if (isPathOrSubpath(normalizedTmpdirValue, normalize2("/tmp")) || isPathOrSubpath(normalizedTmpdirValue, normalize2("/var/tmp")) || isPathOrSubpath(normalizedTmpdirValue, sysTmpdir)) {
16101605
return false;
16111606
}
16121607
return true;
@@ -1615,7 +1610,7 @@ function isPathOrSubpath(path, basePath) {
16151610
if (path === basePath) {
16161611
return true;
16171612
}
1618-
const baseWithSlash = basePath.endsWith("/") ? basePath : `${basePath}/`;
1613+
const baseWithSlash = basePath.endsWith(sep2) ? basePath : `${basePath}${sep2}`;
16191614
return path.startsWith(baseWithSlash);
16201615
}
16211616

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
"build:schema": "bun run scripts/build-schema.ts",
1818
"clean": "rm -rf dist",
1919
"check": "bun run lint && bun run typecheck && bun run knip && bun run sg:scan && AGENT=1 bun test --coverage",
20+
"check:ci": "bun run lint:ci && bun run typecheck && bun run knip && bun run sg:scan && AGENT=1 bun test --coverage --coverage-reporter=lcov",
2021
"lint": "biome check --write",
22+
"lint:ci": "biome ci .",
2123
"typecheck": "tsc --noEmit",
2224
"knip": "knip --production",
2325
"sg:scan": "ast-grep scan",

‎src/core/analyze/tmpdir.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { tmpdir } from 'node:os';
2+
import { normalize, sep } from 'node:path';
23

34
export function isTmpdirOverriddenToNonTemp(envAssignments: Map<string, string>): boolean {
45
if (!envAssignments.has('TMPDIR')) {
@@ -11,12 +12,14 @@ export function isTmpdirOverriddenToNonTemp(envAssignments: Map<string, string>)
1112
return true;
1213
}
1314

15+
const normalizedTmpdirValue = normalize(tmpdirValue);
16+
1417
// Check if it's a known temp path (exact match or subpath)
15-
const sysTmpdir = tmpdir();
18+
const sysTmpdir = normalize(tmpdir());
1619
if (
17-
isPathOrSubpath(tmpdirValue, '/tmp') ||
18-
isPathOrSubpath(tmpdirValue, '/var/tmp') ||
19-
isPathOrSubpath(tmpdirValue, sysTmpdir)
20+
isPathOrSubpath(normalizedTmpdirValue, normalize('/tmp')) ||
21+
isPathOrSubpath(normalizedTmpdirValue, normalize('/var/tmp')) ||
22+
isPathOrSubpath(normalizedTmpdirValue, sysTmpdir)
2023
) {
2124
return false;
2225
}
@@ -32,7 +35,7 @@ function isPathOrSubpath(path: string, basePath: string): boolean {
3235
if (path === basePath) {
3336
return true;
3437
}
35-
// Ensure basePath ends with / for proper prefix matching
36-
const baseWithSlash = basePath.endsWith('/') ? basePath : `${basePath}/`;
38+
// Ensure basePath ends with the platform separator for proper prefix matching.
39+
const baseWithSlash = basePath.endsWith(sep) ? basePath : `${basePath}${sep}`;
3740
return path.startsWith(baseWithSlash);
3841
}

‎src/core/shell.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,7 @@ export function splitShellCommands(command: string): string[][] {
2020
let i = 0;
2121

2222
while (i < tokens.length) {
23-
const token = tokens[i];
24-
if (token === undefined) {
25-
i++;
26-
continue;
27-
}
28-
23+
const token = tokens[i] as ParseEntry;
2924
if (isOperator(token)) {
3025
if (current.length > 0) {
3126
segments.push(current);
@@ -182,9 +177,6 @@ function parseEnvAssignment(token: string): { name: string; value: string } | nu
182177
return null;
183178
}
184179
const eqIdx = token.indexOf('=');
185-
if (eqIdx < 0) {
186-
return null;
187-
}
188180
return { name: token.slice(0, eqIdx), value: token.slice(eqIdx + 1) };
189181
}
190182

‎tests/bin/doctor/system-info.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ describe('defaultVersionFetcher', () => {
8080
expect(result).toBeNull();
8181
});
8282

83+
test('returns null when spawn throws synchronously for invalid command input', async () => {
84+
const result = await defaultVersionFetcher(['\u0000']);
85+
expect(result).toBeNull();
86+
});
87+
8388
test('returns version for existing commands', async () => {
8489
const result = await defaultVersionFetcher(['bun', '--version']);
8590
expect(result).not.toBeNull();

‎tests/bin/explain/command.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,12 @@ describe('explainCommand guard parity fixes', () => {
333333
expect(result.result).toBe('allowed');
334334
});
335335

336+
test('Fix #3: TMPDIR traversal override blocks rm', () => {
337+
const result = explainCommand('TMPDIR=/tmp/../root rm -rf $TMPDIR/foo', { cwd: '/tmp' });
338+
expect(result.result).toBe('blocked');
339+
expect(result.reason).toContain('rm -rf');
340+
});
341+
336342
test('Fix #4: fallback scan finds embedded git in non-head position', () => {
337343
const result = explainCommand('nice git reset --hard');
338344
expect(result.result).toBe('blocked');

‎tests/core/analyze/analyze-coverage.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,14 @@ describe('analyzeCommand (coverage)', () => {
136136
expect(result).toBeNull();
137137
});
138138

139+
test('TMPDIR traversal override blocks $TMPDIR', () => {
140+
const result = analyzeCommand('TMPDIR=/tmp/../root rm -rf $TMPDIR/test-dir', {
141+
cwd: '/tmp',
142+
config: EMPTY_CONFIG,
143+
});
144+
expect(result?.reason).toContain('rm -rf');
145+
});
146+
139147
test('xargs child git command is analyzed', () => {
140148
const result = analyzeCommand('xargs git reset --hard', {
141149
cwd: '/tmp',

0 commit comments

Comments
 (0)