From f6582dfb5e8aef24a8faddc37c208c1189cfab33 Mon Sep 17 00:00:00 2001 From: Yuval Dinodia <102706514+yetval@users.noreply.github.com> Date: Fri, 17 Jul 2026 01:22:01 -0400 Subject: [PATCH] fix(file-transfer): denyPaths does not deny listing the denied directory itself (#109233) * fix(file-transfer): deny listing the directory a denyPaths rule names A denyPaths entry like "**/.ssh/**" did not deny dir.list on the denied directory itself, so listing /home/me/.ssh returned its full contents, including filenames such as id_rsa and authorized_keys. Only paths strictly under the directory were denied. The deny gate matched the requested path against the globs as a plain string, and a trailing /** requires something after the separator, so the bare directory never matched. Every command routes its path through evaluateFilePolicy, but dir.list is where it is observable, since its payload is the directory's own contents. dir.fetch was incidentally safe because it rechecks each returned entry, and those entries do match; dir.list has no such recheck. Match the deny globs against the directory form of the path at that one gate. The check stays deny-side only, so it can fail closed but never open. * test(file-transfer): cover denied directory path forms --------- Co-authored-by: Peter Steinberger --- .../file-transfer/src/shared/policy.test.ts | 34 +++++++++++++++++++ extensions/file-transfer/src/shared/policy.ts | 9 ++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/extensions/file-transfer/src/shared/policy.test.ts b/extensions/file-transfer/src/shared/policy.test.ts index 0b15561db39..ffc47e85f40 100644 --- a/extensions/file-transfer/src/shared/policy.test.ts +++ b/extensions/file-transfer/src/shared/policy.test.ts @@ -201,6 +201,40 @@ describe("evaluateFilePolicy — denyPaths always wins", () => { ); }); + it.each([ + { + label: "the bare denied directory", + requestedPath: path.join(os.homedir(), ".ssh"), + expected: { ok: false, code: "POLICY_DENIED", askable: false }, + }, + { + label: "the denied directory with a trailing separator", + requestedPath: `${path.join(os.homedir(), ".ssh")}/`, + expected: { ok: false, code: "POLICY_DENIED", askable: false }, + }, + { + label: "the bare denied directory on Windows", + requestedPath: "C:\\Users\\me\\.ssh", + expected: { ok: false, code: "POLICY_DENIED", askable: false }, + }, + { + label: "a sibling sharing only the denied directory prefix", + requestedPath: path.join(os.homedir(), ".sshrc"), + expected: { ok: true }, + }, + ])("handles $label", ({ requestedPath, expected }) => { + withConfig({ + n1: { + allowReadPaths: ["/**"], + denyPaths: ["**/.ssh/**"], + }, + }); + expectResultFields( + evaluateFilePolicy({ nodeId: "n1", kind: "read", path: requestedPath }), + expected, + ); + }); + it("denies even with ask=always (denyPaths is hard)", () => { withConfig({ n1: { diff --git a/extensions/file-transfer/src/shared/policy.ts b/extensions/file-transfer/src/shared/policy.ts index c39fae3f740..25ad940db21 100644 --- a/extensions/file-transfer/src/shared/policy.ts +++ b/extensions/file-transfer/src/shared/policy.ts @@ -156,6 +156,13 @@ function matchesAny(target: string, patterns: string[]): boolean { return false; } +function matchesAnyDeny(target: string, patterns: string[]): boolean { + if (matchesAny(target, patterns)) { + return true; + } + return matchesAny(`${target.replace(/[\\/]+$/u, "")}/`, patterns); +} + function resolveNodePolicy( config: FilePolicyConfig, nodeId: string, @@ -262,7 +269,7 @@ export function evaluateFilePolicy(input: { // 1. Deny patterns always win. const denyPatterns = normalizeGlobs(nodeConfig.denyPaths); - if (matchesAny(input.path, denyPatterns)) { + if (matchesAnyDeny(input.path, denyPatterns)) { return { ok: false, code: "POLICY_DENIED",