Skip to content

Commit 20bd9f8

Browse files
committed
test(tools): bound the names-the-parameter match to whole words
cubic found namesParam was a substring scan after stripping non-letters, so it accepted exactly the cases the assertion exists to reject: "Invalid input" satisfied paramName "id" (generic) "projectId cannot have leading …" satisfied paramName "id" (WRONG param) "tableId cannot be '.'" satisfied paramName "table" (WRONG param) "pathological failure" satisfied paramName "path" (substring) A guard naming the wrong identifier therefore satisfied every rejects-by-name assertion across all six suites. The message is now split into letter-only tokens and the parameter must equal a token or a run of adjacent tokens joined. The join keeps prose spellings valid — validateFunctionName reports functionName as "Invalid function name", which is a correct naming, not a near-miss. The run is capped at four tokens and abandoned once longer than the target. All 1523 existing assertions still pass, so no guard was relying on the loose match. namesParam is now exported with its own contract test, because a weakness in it is invisible from every suite it powers: reverting to the substring version fails four of the new cases and nothing else.
1 parent e30c536 commit 20bd9f8

2 files changed

Lines changed: 91 additions & 9 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Contract for `namesParam`, the matcher behind every "names the parameter"
5+
* assertion in the path-safety suites.
6+
*
7+
* It gets its own test because the assertion it powers is only as strong as it
8+
* is, and the previous substring implementation passed all seven suites while
9+
* accepting a message that named the **wrong** parameter. A weakness here is
10+
* invisible everywhere else.
11+
*/
12+
import { describe, expect, it } from 'vitest'
13+
import { namesParam } from '@/tools/__tests__/path-safety'
14+
15+
describe('namesParam', () => {
16+
it.each([
17+
['projectId cannot have leading or trailing whitespace', 'projectId'],
18+
['signRequestId cannot have leading or trailing whitespace', 'signRequestId'],
19+
['bucket cannot contain a path separator', 'bucket'],
20+
['path cannot contain an empty or whitespace-only path segment', 'path'],
21+
['tableId cannot be "." (path traversal is not allowed)', 'tableId'],
22+
['Invalid table: must start with a letter or underscore', 'table'],
23+
])('accepts %j as naming %j', (message, paramName) => {
24+
expect(namesParam(message, paramName)).toBe(true)
25+
})
26+
27+
/**
28+
* A stricter service validator spells the name as prose. Joining adjacent
29+
* tokens is what keeps that a correct naming rather than a near-miss.
30+
*/
31+
it('accepts a prose spelling split across words', () => {
32+
expect(namesParam('Invalid function name: must contain only letters', 'functionName')).toBe(
33+
true
34+
)
35+
})
36+
37+
/** Each of these was accepted by the previous substring implementation. */
38+
it.each([
39+
['a generic message', 'Invalid input', 'id'],
40+
['a message naming a different parameter', 'projectId cannot be ".."', 'id'],
41+
['a longer parameter name containing this one', 'tableId cannot be "."', 'table'],
42+
['the name as a substring of an unrelated word', 'pathological failure', 'path'],
43+
])('rejects %s', (_label, message, paramName) => {
44+
expect(namesParam(message, paramName)).toBe(false)
45+
})
46+
47+
it('rejects an unrelated message outright', () => {
48+
expect(namesParam('Something went wrong', 'path')).toBe(false)
49+
})
50+
})

apps/sim/tools/__tests__/path-safety.ts

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -424,18 +424,50 @@ export function toolsWithoutPathParams(
424424
}
425425

426426
/**
427-
* Normalizes an error message and a parameter name to bare lowercase letters so
428-
* a guard can be credited with naming its parameter however it spells it.
427+
* Reports whether an error message actually names the parameter it is about.
429428
*
430-
* A few parameters are refused by a stricter service-specific validator that
431-
* predates these guards and spells the name in prose — Supabase's
432-
* `functionName` is reported as *"Invalid function name"*. That is an equally
433-
* correct outcome and should still count as naming the offender, so both sides
434-
* are stripped of non-letters before the comparison.
429+
* The match is bounded to whole words rather than a substring scan, because a
430+
* substring scan quietly accepts the two things this assertion exists to
431+
* reject. With a bare `strip(message).includes(strip(paramName))`:
432+
*
433+
* ```
434+
* "Invalid input" satisfied paramName "id" (generic message)
435+
* "projectId cannot have leading …" satisfied paramName "id" (names the WRONG parameter)
436+
* "tableId cannot be '.'" satisfied paramName "table" (names the WRONG parameter)
437+
* "pathological failure" satisfied paramName "path" (substring of a longer word)
438+
* ```
439+
*
440+
* So the message is split into letter-only tokens, and the parameter matches
441+
* only if it equals a token or a run of **adjacent** tokens joined. The join is
442+
* what keeps prose spellings working: a stricter service validator reports
443+
* `functionName` as *"Invalid function name"*, which is `function` + `name`,
444+
* and that is a correct naming rather than a near-miss. The run is capped at
445+
* four tokens and abandoned once it is longer than the target, so this stays
446+
* linear in the message length.
447+
*
448+
* Exported so its own contract can be pinned in `path-safety-matcher.test.ts`;
449+
* the loose version passed every suite while accepting all four cases above.
435450
*/
436-
function namesParam(message: string, paramName: string): boolean {
451+
export function namesParam(message: string, paramName: string): boolean {
437452
const strip = (text: string) => text.toLowerCase().replaceAll(/[^a-z]/g, '')
438-
return strip(message).includes(strip(paramName))
453+
const target = strip(paramName)
454+
if (!target) return false
455+
456+
const tokens = message
457+
.toLowerCase()
458+
.split(/[^a-z]+/)
459+
.filter(Boolean)
460+
461+
for (let start = 0; start < tokens.length; start++) {
462+
let joined = ''
463+
for (let end = start; end < tokens.length && end < start + 4; end++) {
464+
joined += tokens[end]
465+
if (joined === target) return true
466+
if (joined.length > target.length) break
467+
}
468+
}
469+
470+
return false
439471
}
440472

441473
export interface TraversalOptions {

0 commit comments

Comments
 (0)