fix: contain file-explorer writes and browse reachable out-of-base binds (#1465)

The file-explorer editor save resolved a dangling symlink leaf to the link
path and wrote through it with a plain writeFile, which followed the link and
created a file outside the bind/stack root. Reject a resolved leaf that is
itself a symlink (mirroring the managed-stack guard) and promote the save
through the atomic stage-and-rename helper, so the editor save matches its
documented atomicity and can never leave a partial file or land outside the
root.

Bind-root discovery reported every source outside the compose base as
unreachable without probing it, so a config directory mounted into both the
app and the Sencho container was wrongly non-browsable. Probe the declared
source as Sencho actually sees it; dangerous host roots, docker-socket mounts,
and managed-area overlaps stay blocked, and the dangerous classification also
reads the literal declared source so it holds across platforms.
This commit is contained in:
Anso
2026-06-26 09:23:50 -04:00
committed by GitHub
parent 315e8b6379
commit 1c82e3e1d4
4 changed files with 156 additions and 32 deletions
@@ -218,6 +218,64 @@ describe.skipIf(isWindows)('FileSystemService symlink-escape: dangling (broken)
});
});
// ── Scoped file-explorer writes (bind roots) must reject a dangling symlink ──
// leaf the same way the legacy managed-stack path does: the editor save sink
// (writeStackFileIfUnchanged) resolves through resolveSafePathWithin, which
// must not hand back a link path a follow-on write would traverse out of the
// root. The escape only triggers on first save (no expected mtime).
describe.skipIf(isWindows)('FileSystemService scoped-root: dangling symlink leaf write', () => {
let tmpBase: string;
let composeDir: string;
let rootDir: string;
let externalTarget: string;
beforeEach(async () => {
tmpBase = await fs.mkdtemp(path.join(os.tmpdir(), 'sencho-scopedangle-'));
composeDir = path.join(tmpBase, 'compose');
rootDir = path.join(tmpBase, 'bindroot');
externalTarget = path.join(tmpBase, 'outside');
await fs.mkdir(composeDir, { recursive: true });
await fs.mkdir(rootDir, { recursive: true });
// The escape target dir exists; only the leaf file is missing, so the leaf
// symlink is dangling (realpath ENOENT) but a writeFile would create it.
await fs.mkdir(externalTarget, { recursive: true });
mockState.composeDir = composeDir;
});
afterEach(async () => {
await fs.rm(tmpBase, { recursive: true, force: true });
});
it('rejects a first-save through a dangling symlink leaf and writes nothing outside the root', async () => {
await fs.symlink(path.join(externalTarget, 'pwned.conf'), path.join(rootDir, 'leak.conf'));
const svc = FileSystemService.getInstance();
await expect(
svc.writeStackFileIfUnchanged(STACK, 'leak.conf', 'PWNED=1\n', null, { rootAbsDir: rootDir }),
).rejects.toMatchObject({ code: 'SYMLINK_ESCAPE' });
await expect(fs.access(path.join(externalTarget, 'pwned.conf'))).rejects.toMatchObject({ code: 'ENOENT' });
});
it('rejects a scoped read through a dangling symlink leaf (shared resolver guard)', async () => {
await fs.symlink(path.join(externalTarget, 'secret.conf'), path.join(rootDir, 'peek.conf'));
const svc = FileSystemService.getInstance();
await expect(
svc.readStackFile(STACK, 'peek.conf', undefined, { scope: { rootAbsDir: rootDir } }),
).rejects.toMatchObject({ code: 'SYMLINK_ESCAPE' });
});
it('writes a normal scoped file atomically (no leftover staging file)', async () => {
const svc = FileSystemService.getInstance();
const res = await svc.writeStackFileIfUnchanged(STACK, 'app.conf', 'key=value\n', null, { rootAbsDir: rootDir });
expect(res.ok).toBe(true);
expect(await fs.readFile(path.join(rootDir, 'app.conf'), 'utf-8')).toBe('key=value\n');
// The atomic helper stages into a sibling .sencho-tmp-* file and renames it
// away; nothing transient should remain in the root after a clean write.
const entries = await fs.readdir(rootDir);
expect(entries.filter((e) => e.includes('sencho-tmp'))).toHaveLength(0);
expect(entries).toEqual(['app.conf']);
});
});
// ── The compose root itself is a symlink: must NOT be a false positive ───────
describe.skipIf(isWindows)('FileSystemService symlink-escape: symlinked compose root is allowed', () => {
let tmpBase: string;
@@ -117,6 +117,35 @@ describe('StackFileRootsService.listRoots', () => {
expect(bind?.browsable).toBe(false);
});
it('classifies a reachable absolute bind outside the compose base as browsable and writable', async () => {
// A config directory mounted into both the app and the Sencho container can
// legitimately live outside the compose base. When Sencho can stat it, the
// root must be fully browsable/editable, not silently dropped as unreachable.
const outside = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), 'sfr-ext-')));
try {
stub({ rendered: renderModel({ web: [{ type: 'bind', source: outside, target: '/config', read_only: false }] }) });
const roots = await StackFileRootsService.getInstance(1).listRoots(STACK, { fresh: true });
const bind = roots.find((r) => r.kind === 'bind');
expect(bind?.accessible).toBe(true);
expect(bind?.browsable).toBe(true);
expect(bind?.writable).toBe(true);
expect(bind?.managedSourceOverlap).toBe(false);
expect(bind?.hostPathOrName).toBe(outside);
} finally {
await fs.rm(outside, { recursive: true, force: true }).catch(() => {});
}
});
it('marks an unreachable absolute bind outside the compose base as non-browsable', async () => {
const outsideMissing = path.join(os.tmpdir(), 'sfr-ext-absent-xyz-12345');
stub({ rendered: renderModel({ web: [{ type: 'bind', source: outsideMissing, target: '/config' }] }) });
const roots = await StackFileRootsService.getInstance(1).listRoots(STACK, { fresh: true });
const bind = roots.find((r) => r.kind === 'bind');
expect(bind?.accessible).toBe(false);
expect(bind?.browsable).toBe(false);
expect(bind?.warning).toBeTruthy();
});
it('blocks a dangerous host bind (/etc) and never exposes it as browsable', async () => {
stub({ rendered: renderModel({ web: [{ type: 'bind', source: '/etc', target: '/host-etc' }] }) });
const roots = await StackFileRootsService.getInstance(1).listRoots(STACK, { fresh: true });
@@ -125,6 +154,20 @@ describe('StackFileRootsService.listRoots', () => {
expect(bind?.browsable).toBe(false);
});
it('blocks a dangerous declared source even when realpath rewrites it to a benign canonical', async () => {
// Guards the dangerousSource term: realpath can rewrite a dangerous POSIX
// source to a benign-looking canonical (a non-existent POSIX path resolves
// drive-prefixed on a non-Linux host), so isDangerousHostPath(canonical)
// alone would miss it. The classification must also read the literal source.
vi.spyOn(fs, 'realpath').mockResolvedValue('/srv/benign-canonical' as never);
vi.spyOn(fs, 'stat').mockResolvedValue({ isDirectory: () => true } as never);
stub({ rendered: renderModel({ web: [{ type: 'bind', source: '/etc', target: '/host-etc' }] }) });
const roots = await StackFileRootsService.getInstance(1).listRoots(STACK, { fresh: true });
const bind = roots.find((r) => r.kind === 'bind');
expect(bind?.dangerous).toBe(true);
expect(bind?.browsable).toBe(false);
});
it('folds a bind equal to the stack dir into stack-source (no second editable root)', async () => {
stub({ rendered: renderModel({ web: [{ type: 'bind', source: stackDir, target: '/app' }] }) });
const roots = await StackFileRootsService.getInstance(1).listRoots(STACK, { fresh: true });