From bf70150eb2df8070024e5bdf08e4aa08959e2735 Mon Sep 17 00:00:00 2001 From: xsf <45911614+xsfX20@users.noreply.github.com> Date: Tue, 22 Sep 2026 02:40:16 +0800 Subject: [PATCH] fix(gateguard): sanitize dangerous invisible unicode in denial paths (#3103) * fix(gateguard): sanitize dangerous invisible unicode in denial paths sanitizePath only stripped control characters and bidi overrides, so the 394 codepoints that the repo-wide unicode safety policy classifies as dangerous-invisible (zero-width U+200B-200D, U+2060, U+2061-2064, variation selectors U+FE00-FE0F, tag block U+E0000-E007F, Hangul fillers, U+FEFF, U+180E, U+3164) plus the U+2028/U+2029 line and paragraph separators passed verbatim into Edit/Write denial messages. Invisible characters in a quoted file path let a malicious path look clean to a human reviewer. Align sanitizePath with the CI policy list and cover it with a hook-output regression test that asserts no dangerous invisible character survives into the denial reason while visible path text stays intact. * refactor(gateguard): name the sanitizePath unicode policy constants Address review feedback: move the code points and ranges out of sanitizePath into descriptive constants mirroring scripts/ci/check-unicode-safety.js, so the policy is auditable in one place. Also seed the regression test input with U+2029 so the paragraph-separator assertion is exercised rather than vacuous. * fix(gateguard): sanitize C1 control characters in denial paths Follow-up on the sanitizePath policy alignment: the Unicode C1 control block (U+0080..U+009F) is invisible in every renderer like the ASCII controls below 0x20, but the strip only covered the ASCII range. Add C1_CONTROLS to the policy constants and seed U+0091 into the regression test input. --- scripts/hooks/gateguard-fact-force.js | 56 +++++++++++++++++++++--- tests/hooks/gateguard-fact-force.test.js | 28 ++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 53d8424fe..6756a0b79 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -1262,16 +1262,62 @@ function isChecked(key) { // --- Sanitize file path against injection --- +// Unicode policy for sanitizePath, mirroring the repo-wide dangerous set in +// scripts/ci/check-unicode-safety.js. Named so the ranges stay auditable and +// drift against the CI policy is visible in one place. +const ASCII_CONTROL_MAX = 0x1f; +const ASCII_DELETE = 0x7f; +const C1_CONTROLS = [0x80, 0x9f]; // Unicode C1 control block (U+0080..U+009F) +const BIDI_MARKS = [0x200e, 0x200f]; // LRM/RLM +const BIDI_EMBEDDINGS = [0x202a, 0x202e]; // LRE..PDF +const BIDI_ISOLATES = [0x2066, 0x2069]; // LRI..PDI +const ZERO_WIDTHS = [0x200b, 0x200d]; // ZWSP..ZWJ +const WORD_JOINER = 0x2060; +const BYTE_ORDER_MARK = 0xfeff; +const VARIATION_SELECTORS = [0xfe00, 0xfe0f]; +const VARIATION_SUPPLEMENTS = [0xe0100, 0xe01ef]; // MONGOLIAN..TAGS (VS17..VS256) +const TAG_BLOCK = [0xe0000, 0xe007f]; // ASCII-smuggling tag characters +const MONGOLIAN_VOWEL_SEPARATOR = 0x180e; +const HANGUL_CHOSEONG_FILLER = 0x115f; +const HANGUL_JUNGSEONG_FILLER = 0x1160; +const HANGUL_FILLER = 0x3164; +const INVISIBLE_MATH_OPERATORS = [0x2061, 0x2064]; // FUNCTION APPLICATION..INVISIBLE PLUS +const LINE_SEPARATOR = 0x2028; +const PARAGRAPH_SEPARATOR = 0x2029; +const SANITIZED_PATH_MAX_LENGTH = 500; + +function inRange(code, [lo, hi]) { + return code >= lo && code <= hi; +} + function sanitizePath(filePath) { - // Strip control chars (including null), bidi overrides, and newlines + // Strip control chars (including null), bidi overrides, separators, + // and the dangerous invisible characters defined by the constants + // above (mirroring scripts/ci/check-unicode-safety.js), so a denial + // message cannot carry content a human reviewer cannot see. let sanitized = ''; for (const char of String(filePath || '')) { const code = char.codePointAt(0); - const isAsciiControl = code <= 0x1f || code === 0x7f; - const isBidiOverride = (code >= 0x200e && code <= 0x200f) || (code >= 0x202a && code <= 0x202e) || (code >= 0x2066 && code <= 0x2069); - sanitized += isAsciiControl || isBidiOverride ? ' ' : char; + const isAsciiControl = + code <= ASCII_CONTROL_MAX || code === ASCII_DELETE || inRange(code, C1_CONTROLS); + const isBidiOverride = + inRange(code, BIDI_MARKS) || inRange(code, BIDI_EMBEDDINGS) || inRange(code, BIDI_ISOLATES); + const isUnicodeSeparator = code === LINE_SEPARATOR || code === PARAGRAPH_SEPARATOR; + const isDangerousInvisible = + inRange(code, ZERO_WIDTHS) || + code === WORD_JOINER || + code === BYTE_ORDER_MARK || + inRange(code, VARIATION_SELECTORS) || + inRange(code, VARIATION_SUPPLEMENTS) || + inRange(code, TAG_BLOCK) || + code === MONGOLIAN_VOWEL_SEPARATOR || + code === HANGUL_CHOSEONG_FILLER || + code === HANGUL_JUNGSEONG_FILLER || + code === HANGUL_FILLER || + inRange(code, INVISIBLE_MATH_OPERATORS); + sanitized += isAsciiControl || isBidiOverride || isUnicodeSeparator || isDangerousInvisible ? ' ' : char; } - return sanitized.trim().slice(0, 500); + return sanitized.trim().slice(0, SANITIZED_PATH_MAX_LENGTH); } function normalizeForMatch(value) { diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 399cca83b..9e6a18334 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -3369,6 +3369,34 @@ function runTests() { console.error(` [cleanup] failed to remove ${stateDir}: ${err.message}`); } + // --- sanitizePath dangerous invisible unicode regression --- + clearState(); + if ( + test('sanitizePath strips CI-defined dangerous invisible unicode from denial paths', () => { + const file_path = + '/src/eu2028\u2028eu2029\u2029app.js\u200bhidden\u2060name\ufefftail\u3164x\u0091c1.js'; + const input = { + tool_name: 'Edit', + tool_input: { file_path, old_string: 'foo', new_string: 'bar' } + }; + const result = runHook(input); + const output = parseOutput(result.stdout); + const reason = String( + output && output.hookSpecificOutput + ? output.hookSpecificOutput.permissionDecisionReason + : '' + ); + for (const bad of ['\u2028', '\u2029', '\u200b', '\u2060', '\ufeff', '\u3164', '\u0091']) { + assert.ok(!reason.includes(bad), `denial reason must not carry U+${bad.codePointAt(0).toString(16)} (${bad})`); + } + assert.ok(reason.includes('app.js'), 'visible path text must remain'); + }) + ) { + passed++; + } else { + failed++; + } + console.log(`\n ${passed} passed, ${failed} failed\n`); process.exit(failed > 0 ? 1 : 0); }