Skip to content

Commit f9ea0a6

Browse files
committed
test(tools): fail when a guard rejects a value it must render inert
cubic found the fourth instance of the same pattern: the renders-inert cases caught any throw and returned, so none of the assertions ran. The suite proved only that values which build stay inert — a guard that over-tightened and rejected one it should have encoded passed silently. The surrounding-whitespace case for ordinary ids had the same hole. A throw is now a failure unless the parameter is named in strictlyValidated, which is exactly Supabase table and functionName: validateDatabaseIdentifier and validateFunctionName predate these guards and legitimately refuse values the shared guards only render inert. Measured rather than assumed — those ten pairs are the only ones that reject any MUST_NOT_RESHAPE value. Verified non-vacuous by over-tightening strictUrlPathSegment to reject '#': three box_sign cases fail where they previously passed.
1 parent 796eefe commit f9ea0a6

2 files changed

Lines changed: 38 additions & 2 deletions

File tree

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,20 @@ export interface TraversalOptions {
425425
* rejection cannot quietly regress into a trim.
426426
*/
427427
rejectsSurroundingWhitespace?: readonly string[]
428+
/**
429+
* Parameters guarded by a stricter, service-specific validator that predates
430+
* these path guards — Supabase's `table` and `column` go through
431+
* `validateDatabaseIdentifier`, `functionName` through `validateFunctionName`.
432+
*
433+
* Those legitimately refuse values the shared guards merely render inert
434+
* (`abc#fragment` is a fine URL segment but not a SQL identifier), so a throw
435+
* from them is a correct outcome. Everywhere else a throw is a **failure**:
436+
* `MUST_NOT_RESHAPE` values must actually reach the wire encoded, and a guard
437+
* that over-tightens and rejects one is a regression the suite has to catch.
438+
* Listing the exceptions by name is what keeps "tolerated" from silently
439+
* becoming "untested".
440+
*/
441+
strictlyValidated?: readonly string[]
428442
}
429443

430444
/** Asserts the traversal invariant for one (tool, parameter) pair. */
@@ -435,6 +449,7 @@ export function itResistsTraversal(
435449
basePath,
436450
preservesWhitespace = false,
437451
rejectsSurroundingWhitespace = [],
452+
strictlyValidated = [],
438453
}: TraversalOptions
439454
): void {
440455
const baselinePath = buildUrl(tool, paramName, PROBE_ID, context).pathname
@@ -473,7 +488,18 @@ export function itResistsTraversal(
473488
let url: URL
474489
try {
475490
url = buildUrl(tool, paramName, value, context)
476-
} catch {
491+
} catch (error) {
492+
/**
493+
* A throw here is only acceptable from a parameter with a stricter
494+
* pre-existing validator. Otherwise the value was supposed to survive
495+
* encoded, and swallowing the rejection would hide a guard that has
496+
* over-tightened — the suite would then prove only that values which
497+
* *build* stay inert, which is not the property claimed.
498+
*/
499+
expect(
500+
strictlyValidated.includes(paramName),
501+
`${paramName} rejected ${JSON.stringify(value)}, which must be rendered inert: ${getErrorMessage(error, 'unknown error')}`
502+
).toBe(true)
477503
return
478504
}
479505

@@ -561,7 +587,11 @@ export function itResistsTraversal(
561587
let url: URL
562588
try {
563589
url = buildUrl(tool, paramName, padded, context)
564-
} catch {
590+
} catch (error) {
591+
expect(
592+
strictlyValidated.includes(paramName),
593+
`${paramName} rejected a padded value without being a strictly-validated parameter: ${getErrorMessage(error, 'unknown error')}`
594+
).toBe(true)
565595
return
566596
}
567597

apps/sim/tools/supabase/path_safety.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ describe('supabase path traversal safety', () => {
101101
origin: ORIGIN,
102102
basePath: BASE_PATH,
103103
preservesWhitespace: param.paramName === 'path',
104+
/**
105+
* `table` and `functionName` are refused by `validateDatabaseIdentifier`
106+
* and `validateFunctionName`, which predate these guards and legitimately
107+
* reject values the shared guards only render inert.
108+
*/
109+
strictlyValidated: ['table', 'functionName'],
104110
})
105111
})
106112

0 commit comments

Comments
 (0)