From 349ee1f905da68a5d520b4762909bfb29ffbe282 Mon Sep 17 00:00:00 2001 From: Anso Date: Sat, 27 Jun 2026 00:43:29 -0400 Subject: [PATCH] fix: contain file-explorer binds that reach symlinked Sencho-managed paths (#1489) The file-explorer overlap check canonicalized declared bind sources (it realpaths the source) but compared them against the Sencho-managed paths (compose base, data dir, application root, OS temp root, upload spool, Trivy binary, Trivy cache) using path.resolve only. A managed path that is itself a symlink, for example a relocated Trivy binary whose configured path links to a real binary elsewhere, was therefore compared by its symlink path. A bind to the symlink target's real directory did not register as a managed overlap and became browsable and writable, so a stack editor could overwrite the real binary a later pre or post deploy scan executes, or reach transient registry credentials under a symlinked temporary root. Resolve each managed path to its canonical target and keep both the configured path and the realpath target in the overlap set, mirroring the bind-side canonicalization. Containment only ever expands, so existing deployments and fresh installs are unaffected. A missing managed path (the common fresh-install case) is tolerated: the configured path still anchors containment, and any non-ENOENT realpath failure is logged rather than collapsing discovery. Adds two tests: a regression test that fails when a symlinked managed Trivy path is compared without canonicalization, and a fresh-install test that an absent managed path leaves a legitimate external bind browsable. --- .../stack-file-roots-service.test.ts | 50 +++++++++++++++++++ backend/src/services/StackFileRootsService.ts | 41 ++++++++++++--- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/backend/src/__tests__/stack-file-roots-service.test.ts b/backend/src/__tests__/stack-file-roots-service.test.ts index 77bf5b3b..b3f951ca 100644 --- a/backend/src/__tests__/stack-file-roots-service.test.ts +++ b/backend/src/__tests__/stack-file-roots-service.test.ts @@ -327,6 +327,56 @@ describe('StackFileRootsService.listRoots', () => { } }); + it('suppresses a bind to the real target of a symlinked managed Trivy binary (canonical managed root)', async () => { + // TRIVY_BIN (and the other managed paths) may itself be a symlink, e.g. + // /opt/trivy -> /srv/tool-target/trivy. Bind sources are canonicalized before + // the overlap check, so the managed paths must be canonicalized too. Without + // it, a bind to the symlink target's real directory does not overlap the + // configured symlink path, and a stack editor could overwrite the real binary + // a later scan executes. realpath is the only canonicalization seam, so stub + // it to resolve the configured symlink to the real binary path inside the + // bound directory; identity for every other path leaves the real + // baseDir/app/tmp comparisons intact. + const targetDir = await fs.realpath(await fs.mkdtemp(path.join(REAL_TMP, 'sfr-symtarget-'))); + const realBinary = path.join(targetDir, 'trivy'); + const symlinkBin = path.join(REAL_TMP, 'sfr-symlinked-trivy-bin'); + process.env.TRIVY_BIN = symlinkBin; + vi.spyOn(fs, 'realpath').mockImplementation((async (p: string) => + path.resolve(p) === path.resolve(symlinkBin) ? realBinary : p) as typeof fs.realpath); + try { + // Bind the real target directory, NOT the symlink's directory: only the + // canonicalized managed root catches this overlap. + stub({ rendered: renderModel({ web: [{ type: 'bind', source: targetDir, target: '/opt/trivy', read_only: false }] }) }); + const roots = await StackFileRootsService.getInstance(1).listRoots(STACK, { fresh: true }); + const bind = roots.find((r) => r.kind === 'bind'); + expect(bind?.managedSourceOverlap).toBe(true); + expect(bind?.browsable).toBe(false); + expect(bind?.writable).toBe(false); + } finally { + await fs.rm(targetDir, { recursive: true, force: true }).catch(() => {}); + } + }); + + it('keeps a legitimate external bind browsable when a configured managed path is absent (ENOENT realpath tolerated)', async () => { + // A relocated TRIVY_BIN (or upload spool) routinely points at a path that does + // not exist yet on a fresh install, so realpath on a configured managed path + // throws ENOENT every discovery. That must be tolerated: the configured path + // still anchors containment and an unrelated external bind stays browsable, + // rather than the realpath failure collapsing discovery to stack-source only. + process.env.TRIVY_BIN = path.join(REAL_TMP, 'sfr-absent-trivy-bin-xyz-12345'); + const outside = await fs.realpath(await fs.mkdtemp(path.join(REAL_TMP, 'sfr-legit-'))); + 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?.managedSourceOverlap).toBe(false); + expect(bind?.browsable).toBe(true); + expect(bind?.writable).toBe(true); + } finally { + await fs.rm(outside, { recursive: true, force: true }).catch(() => {}); + } + }); + it('resolves a named volume by its Docker name (not the compose key) and inspects that name', async () => { const inspected: string[] = []; stub({ diff --git a/backend/src/services/StackFileRootsService.ts b/backend/src/services/StackFileRootsService.ts index d4ba66b4..73582aad 100644 --- a/backend/src/services/StackFileRootsService.ts +++ b/backend/src/services/StackFileRootsService.ts @@ -167,13 +167,39 @@ function resolveAppRoot(): string { * The managed Trivy install and cache default under the data dir, so they are * already covered unless an env override moves them elsewhere. */ -function resolveManagedRoots(baseDir: string): string[] { - const roots = [path.resolve(baseDir), resolveDataDir(), resolveAppRoot(), path.resolve(os.tmpdir())]; - const add = (value: string | undefined): void => { if (value) roots.push(path.resolve(value)); }; +async function resolveManagedRoots(baseDir: string): Promise { + const configured = [path.resolve(baseDir), resolveDataDir(), resolveAppRoot(), path.resolve(os.tmpdir())]; + const add = (value: string | undefined): void => { if (value) configured.push(path.resolve(value)); }; add(process.env.SENCHO_UPLOAD_DIR); add(process.env.TRIVY_BIN); add(process.env.TRIVY_CACHE_DIR); - return roots; + + // Bind sources are canonicalized before the overlap check (probeBindRootAccess + // realpaths the declared source), so a managed path that is itself a symlink + // must be canonicalized too. Otherwise TRIVY_BIN=/opt/trivy -> /srv/tool/trivy + // is only compared as /opt/trivy: a bind to /srv/tool slips past the overlap + // check, letting a stack editor overwrite the real Trivy binary a later scan + // executes (the same gap re-exposes a symlinked upload spool, Trivy cache, or + // OS temp root holding transient registry credentials). Keep both the configured + // path (catches a bind to its literal parent) and the realpath target (catches + // a bind to the symlink target's parent). + const roots = new Set(configured); + for (const p of configured) { + try { + roots.add(await fsPromises.realpath(p)); + } catch (err) { + // ENOENT is expected: a configured-but-absent path (e.g. SENCHO_UPLOAD_DIR, + // or a relocated TRIVY_BIN/TRIVY_CACHE_DIR that env points at but that does + // not exist yet) has no canonical target, and the configured path already + // anchors containment. Anything else (e.g. EACCES) is logged so an operator + // chasing a containment surprise has a trail. + const code = (err as NodeJS.ErrnoException).code; + if (code !== 'ENOENT') { + console.warn('[StackFileRoots] managed-root realpath failed (%s):', code ?? 'unknown', sanitizeForLog(p)); + } + } + } + return [...roots]; } /** A bind source equal to or under one of the dangerous roots (POSIX semantics). */ @@ -398,9 +424,10 @@ export class StackFileRootsService { } } + const managedRoots = await resolveManagedRoots(baseDir); const roots: StackFileRoot[] = []; for (const group of bindByCanonical.values()) { - const root = this.buildBindRoot(group, baseDir, stackDir); + const root = this.buildBindRoot(group, managedRoots, stackDir); if (root) roots.push(root); } for (const group of volByName.values()) { @@ -411,7 +438,7 @@ export class StackFileRootsService { private buildBindRoot( group: BindGroup, - baseDir: string, + managedRoots: string[], stackDir: string, ): StackFileRoot | null { const { canonical } = group; @@ -429,7 +456,7 @@ export class StackFileRootsService { // must never become a browsable/editable root. Compare in both directions so // a mount equal to, inside, or an ancestor of a managed dir is caught. const overlapsManaged = (dir: string): boolean => isPathWithinBase(canonical, dir) || isPathWithinBase(dir, canonical); - const overlap = !inStack && resolveManagedRoots(baseDir).some(overlapsManaged); + const overlap = !inStack && managedRoots.some(overlapsManaged); const dangerous = isDangerousHostPath(canonical) || group.dangerousSource || group.dockerSock; const readonly = group.mounts.every((m) => m.readOnly); const isFile = group.accessible && !group.isDir;