mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-10 23:19:09 +02:00
fix(browse): an untrustable TMPDIR (/, $HOME, a cwd ancestor) never widens the local allowlist
TEMP_DIRS honors os.tmpdir() at daemon start; a daemon launched with TMPDIR=/ would have trusted the whole filesystem for local path validation for its lifetime. Subprocess pins cover /, $HOME, cwd-ancestor rejection and that a benign distinct TMPDIR (the sandbox recipe's $HOME/tmp) stays honored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
ba8edb1d42
commit
5ed46193f1
+12
-1
@@ -19,7 +19,18 @@ export const TEMP_DIR = IS_WINDOWS ? os.tmpdir() : '/tmp';
|
||||
* classic /tmp. Remote file serving (TEMP_ONLY in path-security.ts) stays
|
||||
* pinned to TEMP_DIR alone; this wider set is for LOCAL path validation only.
|
||||
*/
|
||||
export const TEMP_DIRS = [...new Set([TEMP_DIR, os.tmpdir()])];
|
||||
/** A TMPDIR pointed at `/`, the user's home, or a parent of the daemon's cwd
|
||||
* would widen local path validation to that whole subtree for the daemon's
|
||||
* lifetime — treat such a value as misconfiguration and ignore it. */
|
||||
function trustableTmpdir(dir: string): boolean {
|
||||
const resolved = path.resolve(dir);
|
||||
const home = os.homedir();
|
||||
if (resolved === path.parse(resolved).root) return false;
|
||||
if (resolved === home) return false;
|
||||
return !isPathWithin(path.resolve(process.cwd()), resolved) || isPathWithin(resolved, TEMP_DIR);
|
||||
}
|
||||
|
||||
export const TEMP_DIRS = [...new Set([TEMP_DIR, os.tmpdir()].filter((d, i) => i === 0 || trustableTmpdir(d)))];
|
||||
|
||||
/** Check if resolvedPath is within dir, using platform-aware separators. */
|
||||
export function isPathWithin(resolvedPath: string, dir: string): boolean {
|
||||
|
||||
@@ -70,3 +70,33 @@ describe('remote file serving stays pinned to TEMP_DIR alone (no exfil widening)
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('untrustable TMPDIR values never widen the allowlist', () => {
|
||||
// TEMP_DIRS is computed at module load from os.tmpdir(), which honors
|
||||
// TMPDIR — so a daemon launched with TMPDIR=/ or TMPDIR=$HOME must not
|
||||
// trust that subtree for its whole lifetime. Probed via a subprocess so
|
||||
// each case gets a fresh module load.
|
||||
const probe = (tmpdir: string): string[] => {
|
||||
const r = Bun.spawnSync([
|
||||
process.execPath, '-e',
|
||||
"import { TEMP_DIRS } from './browse/src/platform'; console.log(JSON.stringify(TEMP_DIRS));",
|
||||
], { env: { ...process.env, TMPDIR: tmpdir }, cwd: path.resolve(import.meta.dir, '..', '..') });
|
||||
return JSON.parse(r.stdout.toString().trim().split('\n').pop()!);
|
||||
};
|
||||
|
||||
it('TMPDIR=/ and TMPDIR=$HOME collapse to TEMP_DIR alone; a cwd ancestor is rejected too', () => {
|
||||
expect(probe('/')).toEqual([TEMP_DIR]);
|
||||
expect(probe(os.homedir())).toEqual([TEMP_DIR]);
|
||||
// Parent of the daemon cwd (the repo checkout's parent) — rejected.
|
||||
expect(probe(path.resolve(import.meta.dir, '..', '..', '..'))).toEqual([TEMP_DIR]);
|
||||
});
|
||||
|
||||
it('a benign distinct TMPDIR (e.g. $HOME/tmp) is still honored for local paths', () => {
|
||||
const benign = fs.mkdtempSync(path.join(os.homedir(), 'browse-tmp-probe-'));
|
||||
try {
|
||||
expect(probe(benign)).toContain(fs.realpathSync(benign));
|
||||
} finally {
|
||||
fs.rmdirSync(benign);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user