Skip to content

Commit 19d715d

Browse files
committed
fix(path-guard): paren-balanced parser, path.resolve, Rule B sequence, multi-line YAML (sync from template@fbadb76)
1 parent 966dfab commit 19d715d

4 files changed

Lines changed: 410 additions & 102 deletions

File tree

.claude/hooks/path-guard/index.mts

Lines changed: 66 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -184,23 +184,51 @@ const isInScope = (filePath: string): boolean => {
184184
return !EXEMPT_FILE_PATTERNS.some(re => re.test(filePath))
185185
}
186186

187-
const extractPathJoinArgs = (
187+
// Extract every `path.join(...)` and `path.resolve(...)` call from
188+
// the diff and return its argument substring. Uses paren-balancing so
189+
// deeply nested arguments like `path.join(getDir(child(x)), 'Final')`
190+
// are captured correctly — a regex-only approach silently missed any
191+
// argument with 2+ levels of nested parentheses.
192+
const extractPathCalls = (
188193
source: string,
189194
): Array<{ snippet: string; literals: string[] }> => {
190-
// Match `path.join(...)` calls and capture the comma-separated
191-
// argument list. We're not parsing JS — a regex is brittle, but the
192-
// hook is a fast advisory line of defense and the gate runs a more
193-
// thorough whole-repo check at commit time.
194195
const calls: Array<{ snippet: string; literals: string[] }> = []
195-
const callRe = /\bpath\.join\s*\(\s*([^()]*(?:\([^()]*\)[^()]*)*)\)/g
196-
let match: RegExpExecArray | null
197-
while ((match = callRe.exec(source)) !== null) {
198-
const args = match[1]
199-
if (args === undefined) {
196+
const callRe = /\bpath\.(?:join|resolve)\s*\(/g
197+
let m: RegExpExecArray | null
198+
while ((m = callRe.exec(source)) !== null) {
199+
const callStart = m.index
200+
const argsStart = callRe.lastIndex
201+
let depth = 1
202+
let i = argsStart
203+
let inString: '"' | "'" | '`' | null = null
204+
while (i < source.length && depth > 0) {
205+
const ch = source[i]!
206+
if (inString) {
207+
if (ch === '\\') {
208+
i += 2
209+
continue
210+
}
211+
if (ch === inString) {
212+
inString = null
213+
}
214+
} else {
215+
if (ch === '"' || ch === "'" || ch === '`') {
216+
inString = ch
217+
} else if (ch === '(') {
218+
depth += 1
219+
} else if (ch === ')') {
220+
depth -= 1
221+
if (depth === 0) {
222+
break
223+
}
224+
}
225+
}
226+
i += 1
227+
}
228+
if (depth !== 0) {
200229
continue
201230
}
202-
// Pull out string literals from the arg list. Both single and
203-
// double quotes; ignore template-literal interpolations.
231+
const args = source.slice(argsStart, i)
204232
const litRe = /(['"])((?:\\.|(?!\1)[^\\])*)\1/g
205233
const literals: string[] = []
206234
let lit: RegExpExecArray | null
@@ -210,12 +238,13 @@ const extractPathJoinArgs = (
210238
literals.push(value)
211239
}
212240
}
213-
calls.push({ snippet: match[0], literals })
241+
calls.push({ snippet: source.slice(callStart, i + 1), literals })
242+
callRe.lastIndex = i + 1
214243
}
215244
return calls
216245
}
217246

218-
const checkRuleA = (calls: ReturnType<typeof extractPathJoinArgs>): void => {
247+
const checkRuleA = (calls: ReturnType<typeof extractPathCalls>): void => {
219248
for (const call of calls) {
220249
const stages = call.literals.filter(l => STAGE_SEGMENTS.has(l))
221250
const buildRoots = call.literals.filter(l => BUILD_ROOT_SEGMENTS.has(l))
@@ -235,38 +264,37 @@ const checkRuleA = (calls: ReturnType<typeof extractPathJoinArgs>): void => {
235264
}
236265
}
237266

238-
const checkRuleB = (calls: ReturnType<typeof extractPathJoinArgs>): void => {
267+
const checkRuleB = (calls: ReturnType<typeof extractPathCalls>): void => {
239268
for (const call of calls) {
240-
// Look for the sequence: `..` then a known sibling package name
241-
// somewhere in the literal list. The literals come in order from
242-
// the regex, so a sibling appearing AFTER a `..` segment indicates
243-
// cross-package traversal.
244-
let sawDotDot = false
245-
for (const lit of call.literals) {
246-
if (lit === '..') {
247-
sawDotDot = true
248-
continue
249-
}
250-
if (sawDotDot && KNOWN_SIBLING_PACKAGES.has(lit)) {
251-
// Only fire when build-output context appears (otherwise this
252-
// could be a legitimate test fixture path or shared resource).
253-
const hasBuildContext = call.literals.some(
254-
l => BUILD_ROOT_SEGMENTS.has(l) || STAGE_SEGMENTS.has(l),
269+
// A sibling package name *immediately after* a `..` literal (no
270+
// path segment in between) plus build context elsewhere in the
271+
// call indicates cross-package traversal. The previous "sticky
272+
// sawDotDot" form fired falsely when '..' appeared early and an
273+
// unrelated sibling-named segment appeared much later.
274+
const hasBuildContext = call.literals.some(
275+
l => BUILD_ROOT_SEGMENTS.has(l) || STAGE_SEGMENTS.has(l),
276+
)
277+
if (!hasBuildContext) {
278+
continue
279+
}
280+
for (let i = 0; i < call.literals.length - 1; i++) {
281+
if (
282+
call.literals[i] === '..' &&
283+
KNOWN_SIBLING_PACKAGES.has(call.literals[i + 1]!)
284+
) {
285+
const sibling = call.literals[i + 1]!
286+
throw new BlockError(
287+
'B — cross-package path traversal',
288+
`Don't reach into '${sibling}'s build output via \`..\`. Add \`${sibling}: workspace:*\` as a dep and import its \`paths.mts\` via the \`exports\` field. 1 path, 1 reference.`,
289+
call.snippet,
255290
)
256-
if (hasBuildContext) {
257-
throw new BlockError(
258-
'B — cross-package path traversal',
259-
`Don't reach into '${lit}'s build output via \`..\`. Add \`${lit}: workspace:*\` as a dep and import its \`paths.mts\` via the \`exports\` field. 1 path, 1 reference.`,
260-
call.snippet,
261-
)
262-
}
263291
}
264292
}
265293
}
266294
}
267295

268296
const check = (source: string): void => {
269-
const calls = extractPathJoinArgs(source)
297+
const calls = extractPathCalls(source)
270298
if (calls.length === 0) {
271299
return
272300
}

.claude/hooks/path-guard/test/path-guard.test.mts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,50 @@ describe('path-guard — Rule B (cross-package traversal)', () => {
144144
)
145145
assert.equal(code, 0)
146146
})
147+
148+
it('does not fire when .. and sibling are non-adjacent (regression)', () => {
149+
// Earlier regex ran with sticky sawDotDot — once it saw `..` it
150+
// would flag any later sibling-named segment. The fix requires
151+
// the sibling to appear *immediately* after `..`.
152+
const source = `
153+
const x = path.join(PKG, '..', 'cache', 'lief-builder', 'config.json')
154+
`
155+
const { code } = runHook(
156+
'Write',
157+
'packages/foo/scripts/build.mts',
158+
source,
159+
)
160+
assert.equal(code, 0)
161+
})
162+
})
163+
164+
describe('path-guard — paren-balance correctness', () => {
165+
it('detects A through nested function-call args (regression)', () => {
166+
// Old regex used \\([^()]*\\) which only handled one nesting
167+
// level — `path.join(getDir(child(x)), 'build', 'dev', 'Final')`
168+
// silently slipped through. The paren-balancing scanner catches it.
169+
const source = `
170+
const p = path.join(getDir(child(x)), 'build', 'dev', 'out', 'Final')
171+
`
172+
const { code } = runHook(
173+
'Write',
174+
'packages/foo/scripts/build.mts',
175+
source,
176+
)
177+
assert.equal(code, 2)
178+
})
179+
180+
it('detects A in path.resolve() too', () => {
181+
const source = `
182+
const p = path.resolve(PKG, 'build', 'dev', 'out', 'Final', 'bin')
183+
`
184+
const { code } = runHook(
185+
'Write',
186+
'packages/foo/scripts/build.mts',
187+
source,
188+
)
189+
assert.equal(code, 2)
190+
})
147191
})
148192

149193
describe('path-guard — file-type filter', () => {

0 commit comments

Comments
 (0)