diff --git a/.agents/skills/add-block/SKILL.md b/.agents/skills/add-block/SKILL.md
index 7e1d2cc8054..d0e211677ef 100644
--- a/.agents/skills/add-block/SKILL.md
+++ b/.agents/skills/add-block/SKILL.md
@@ -550,6 +550,39 @@ Maps multiple UI fields to a single serialized parameter:
 - ONLY use `canonicalParamId` to link basic/advanced mode alternatives for the same logical parameter
 - Do NOT use it for any other purpose
 
+## Renaming a SubBlock Id Orphans Saved Workflow State
+
+A subBlock `id` is the storage key for every value users have already saved in deployed workflows.
+Renaming it silently orphans that state — the field renders empty and the workflow runs without it.
+
+**Rename the *tool* param and map it; keep the subBlock id.** `tools.config.params` is where the two
+names meet:
+
+```typescript
+// subBlock id stays `timeout` — saved state keeps resolving
+params: (params) => ({ timeoutSeconds: params.timeout, timeout: undefined }),
+```
+
+**This mapper form only works when the renamed tool param is not both `required` and `user-only`.**
+`check-block-registry.ts:222` narrows to exactly that pair, and for those it demands a subBlock whose
+`id` or `canonicalParamId` equals the **tool param id** — a mapper that renames at execution time does
+not satisfy it, because the raw subBlock id is not a valid lookup key once a canonical group resolves
+(`check-block-registry.ts:201`). For a **required user-only** param the two rules cannot both be met by
+a mapper — keeping the tool param name requires sending the reserved key, and clearing it leaves a
+required input undefined — so the only correct move is to **rename the subBlock and record the old id
+in `SUBBLOCK_ID_MIGRATIONS`**, accepting the one-time state migration.
+
+`apps/sim/scripts/check-block-registry.ts` enforces this: it diffs the block definitions and fails
+when a subblock id disappears (`check-block-registry.ts:181`), pointing you at the migration table.
+It separately fails when a required `user-only` tool param has no subBlock whose `id` **or**
+`canonicalParamId` equals it (`check-block-registry.ts:225`), which is the other half of the same
+contract.
+
+For a field that is genuinely gone — not renamed — record it in `SUBBLOCK_ID_MIGRATIONS` in
+`apps/sim/lib/workflows/migrations/subblock-migrations.ts`. A `to` value prefixed with `_removed_`
+(`subblock-migrations.ts:20`, `:109`) means "deleted outright", and is what lets the check pass
+without pretending the value moved somewhere.
+
 ## WandConfig Pattern
 
 Enables AI-assisted field generation.
@@ -607,6 +640,29 @@ tools: {
 }
 ```
 
+### Omitting a key from `tools.config.params` does NOT drop it
+
+`tools.config.params` returns a **patch**, not a replacement. The executor merges it over the raw
+inputs — `apps/sim/executor/handlers/generic/generic-handler.ts:191`:
+
+```typescript
+const transformedParams = blockConfig.tools.config.params(inputs)
+finalInputs = { ...inputs, ...transformedParams }
+```
+
+So a subBlock value reaches the tool even when `params` never mentions its key. This matters when a
+subBlock id collides with a name the shared transport reserves (`timeout`, `proxyUrl`, `method` — see
+**Reserved Parameter Names** in the `add-tools` skill): renaming only the tool-side param leaves the
+old key merging straight back in. Clearing it requires an **explicit** `undefined`:
+
+```typescript
+params: (params) => ({
+  // ✓ Removes the reserved key from the outbound params
+  timeout: undefined,
+  timeoutSeconds: params.timeout,
+}),
+```
+
 ### V2 Versioned Tool Selector
 ```typescript
 import { createVersionedToolSelector } from '@/blocks/utils'
@@ -681,6 +737,24 @@ Nested object outputs (`plan: { id: { type: 'string' }, ... }`) are a **tool-out
 
 If the output shape is unknown because the underlying tool response is undocumented, you MUST tell the user and stop. Unknown is not the same as variable. Never guess block outputs.
 
+### Declared `outputs` do not drive variable resolution
+
+`outputs` is documentation and editor autocomplete — it is **not** the contract `<block.path>`
+references resolve against. `apps/sim/executor/utils/block-reference.ts:239` navigates the **runtime**
+output object, and the declared schema is only consulted at `:242` — `if (value === undefined && schema)`
+— to produce a better error for a path that resolved to nothing.
+
+Two consequences:
+
+- `<block.some.path>` resolves fine even when `some.path` was never declared. An undeclared field is
+  still a live reference someone may have wired up.
+- Changing an output's **shape** therefore breaks saved references that never appeared in `outputs`
+  at all, and no check will tell you. Adding a field is safe; re-nesting, renaming, or wrapping the
+  existing keys is not.
+
+When restructuring a `transformResponse`, spread the raw provider keys **last** so every previously
+reachable path survives alongside the new shape.
+
 ## V2 Block Pattern
 
 When creating V2 blocks (alongside legacy V1):
@@ -1028,6 +1102,9 @@ changes.
 - [ ] Tools.access lists all tool IDs (snake_case)
 - [ ] Tools.config.tool returns correct tool ID (snake_case)
 - [ ] Outputs match tool outputs
+- [ ] No existing subBlock `id` was renamed or removed without a `SUBBLOCK_ID_MIGRATIONS` entry
+- [ ] Restructured outputs still expose every previously reachable runtime key (raw keys spread last)
+- [ ] Where a subBlock id collides with a reserved transport key but means something provider-specific, the reserved key is explicitly cleared with `undefined` in `tools.config.params` (leave it alone when the block genuinely means the transport's timeout/proxy/method)
 - [ ] Block + meta registered in registry-maps.ts (`BLOCK_REGISTRY` / `BLOCK_META_REGISTRY`)
 - [ ] If any tool was added, changed or removed alongside the block: ran `bun run tool-metadata:generate` and committed the artifacts
 - [ ] Ran `bun run scripts/generate-docs.ts`, reviewed the generated diff, and committed the integration catalog changes
diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md
index d3d8e8f64ea..6c2befeb960 100644
--- a/.agents/skills/add-integration/SKILL.md
+++ b/.agents/skills/add-integration/SKILL.md
@@ -930,4 +930,15 @@ requiredScopes: getScopesForService('{service}'),
 11. **Never hardcode scopes** - Use `getScopesForService()` in blocks and `getCanonicalScopesForProvider()` in auth.ts
 12. **Always add scope descriptions** - New scopes must have entries in `SCOPE_DESCRIPTIONS` within `lib/oauth/utils.ts`
 13. **OAuth service IDs need deployment capabilities** - Every visible OAuth integration must resolve through `OAUTH_CLIENT_CAPABILITIES`; shared Google/Microsoft aliases map to their provider capability
-14. **Keep runtime and presentation separate** - Runtime OAuth fields live in `packages/deployment-config/src/env-capabilities.ts`; CLI input modes live in the exhaustively checked `packages/sim-setup/src/capability-config.ts` mapping
+14. **`timeout`, `proxyUrl`, and `method` are reserved** - `apps/sim/tools/request-transport.ts`
+    reads all three off `params` (`:191`, `:198`, `:167`); `timeout` is its own HTTP deadline in
+    **milliseconds**. See **Reserved Parameter Names** in `.agents/skills/add-tools/SKILL.md`
+15. **Never interpolate a param into a URL path raw** - `encodeURIComponent` does not stop `.`/`..`
+    traversal. Use the helpers in `apps/sim/tools/url-path.ts`; see **Path Parameters: Reject
+    Traversal, Never Just Encode** in `.agents/skills/add-tools/SKILL.md`, and add the
+    `path_safety.test.ts` shape from `.agents/skills/validate-integration/SKILL.md`
+16. **Never rename or drop a subBlock `id`** - it is the storage key for deployed workflows. Rename
+    the tool param and map it in `tools.config.params`; see the `add-block` skill
+17. **Omitting a key from `tools.config.params` does not drop it** - the executor merges the patch
+    over the raw inputs, so clearing a key needs an explicit `undefined`; see the `add-block` skill
+18. **Keep runtime and presentation separate** - Runtime OAuth fields live in `packages/deployment-config/src/env-capabilities.ts`; CLI input modes live in the exhaustively checked `packages/sim-setup/src/capability-config.ts` mapping
diff --git a/.agents/skills/add-tools/SKILL.md b/.agents/skills/add-tools/SKILL.md
index 985073b2b5d..ab0a30c5afd 100644
--- a/.agents/skills/add-tools/SKILL.md
+++ b/.agents/skills/add-tools/SKILL.md
@@ -199,6 +199,27 @@ fallback, or caller-controlled `_context` authority.
 - Always explicitly set `required: true` or `required: false`
 - Optional params should have `required: false`
 
+### Reserved Parameter Names
+
+The shared transport reads three names off `params` for its own purposes, before your `request`
+config ever sees them (`apps/sim/tools/request-transport.ts`):
+
+| Param | Read at | What the transport does with it |
+|---|---|---|
+| `timeout` | `request-transport.ts:191` | Outbound HTTP deadline **in milliseconds**, clamped to `getMaxExecutionTimeout()` |
+| `proxyUrl` | `request-transport.ts:198` | Egress proxy URL for the request |
+| `method` | `request-transport.ts:167` | Overrides a *static* `request.method` string (a `method` **function** wins over it) |
+
+Never declare a user-facing param with one of these names unless it means exactly what the transport
+means. The collision is silent and unit-blind: `apps/sim/tools/daytona/execute_command.ts:49`
+declares `timeout` as *"Timeout in seconds (defaults to 10 seconds)"*, so a documented 10-second
+sandbox timeout aborts the HTTP call after **10 milliseconds**.
+
+Give the param a distinct Sim-side name (`timeoutSeconds`, `executionTimeout`, `httpMethod`) and emit
+the provider's spelling from `request.body` or `request.url`. Renaming the tool param is not enough
+on its own if a block already sends the reserved key — see **Omitting a key from `tools.config.params`
+does not drop it** in the `add-block` skill.
+
 ## Resolved Secrets and Provenance Boundaries
 
 - Leave ordinary external API inputs and third-party results unchanged. Add provenance handling only
@@ -219,6 +240,80 @@ fallback, or caller-controlled `_context` authority.
 - Add focused tests for named projection, identical unproven public text, malformed/incomplete
   metadata, metadata stripping, scope isolation, and legacy compatibility where applicable.
 
+## Path Parameters: Reject Traversal, Never Just Encode
+
+`encodeURIComponent` does **not** stop path traversal. `.` and `..` are *unreserved* characters, so
+they survive encoding verbatim, and the WHATWG URL parser that `fetch` uses removes dot segments
+**after** decoding — the percent-encoded spellings included:
+
+```
+new URL('https://x/v1/a/b/..').pathname     // => '/v1/a/'
+new URL('https://x/v1/a/b/%2e%2e').pathname // => '/v1/a/'   (still removed)
+```
+
+A removed segment pops a path segment on a fixed host with the workspace's bearer token still
+attached, including on DELETE routes. Path IDs are typically `visibility: 'user-or-llm'`, so prompt
+injection controls them. Rejection is the only mechanism that closes this; the module note at the top
+of `apps/sim/tools/url-path.ts` states the rule in full.
+
+Never interpolate a param into a request path yourself. Use the helpers in `apps/sim/tools/url-path.ts`.
+
+**Check what your checkout exports before you start.** `safeUrlPathSegment` is on `staging`; the rest
+of this module — `safeUrlPath`, `safeEncodedUrlPathSegment`, `strictUrlPathSegment`,
+`strictEncodedUrlPathSegment` — and the `path_safety.test.ts` suites cited throughout arrive with the
+path-safety sweep. If a helper you need is absent, **add it to `apps/sim/tools/url-path.ts`** with the
+semantics this section specifies — rejection of a bare `.`/`..`, per-segment where the value is
+hierarchical — and never hand-roll a local encoder at the call site. Citations into those files name
+**module and symbol** rather than a line, because they are still being rebased.
+
+| Helper | Use when the parameter is | Trims? |
+|---|---|---|
+| `safeUrlPathSegment` | **One opaque id** — `user_abc`, `12345`, a repo name. Rejects any `/` or `\`: a separator means the caller passed something other than what the parameter addresses. | Yes — surrounding whitespace on a copy-pasted id is transport noise. |
+| `safeUrlPath` | **A real slash-delimited path** the provider documents as such (GitHub `path`, `branch`, `ref`). Splits on `/`, rejects any `.`/`..` segment, percent-encodes each segment, keeps the separators. | **No** — a leading or trailing space is a legal git filename, so trimming would read, update, or *delete* a different file. |
+| `safeEncodedUrlPathSegment` | **One value that may itself contain `/`** but the provider reads as a single parameter (a GitHub label `area/api` in `DELETE .../labels/{name}`). Preserves the separator as `%2F`. | Yes. |
+
+Prefer `safeUrlPathSegment`. Reach for the other two only when the provider documents the parameter
+as slash-bearing — never to make a separator stop erroring on a single-segment id.
+
+GitHub is the worked example, and it settles the case that looks ambiguous: `branch`, `ref`, `base`,
+and `head` take **`safeUrlPath`**, not `safeEncodedUrlPathSegment`. `GET /repos/{o}/{r}/branches/{branch}`
+is greedy on its final parameter, so a branch named `feature/api` is addressed as
+`/branches/feature/api` with a real separator — emitting `%2F` there would 404. The `%2F` form is for
+a parameter the provider reads as one value *and* does not treat as greedy, such as a label name in
+`DELETE .../labels/{name}`. Read the provider's route, not the value's shape.
+
+### Never let a new guard rescue a request that used to fail
+
+If a parameter you are now routing through a helper previously went out raw or through a bare
+`encodeURIComponent`, the helper's trimming is a **behaviour change**, not a tightening: a padded
+value that used to 404 now names a real resource. On a DELETE, a cancel, or a revoke that is a
+destructive action the caller never asked for.
+
+For a newly-trimmed identifier on an irreversible request, use `strictUrlPathSegment` in
+`apps/sim/tools/url-path.ts` — `safeUrlPathSegment` plus a refusal of surrounding whitespace —
+rather than trimming (`strictEncodedUrlPathSegment` is its `%2F`-preserving counterpart). Identifiers
+that were already trimmed before your change keep plain `safeUrlPathSegment`. The full rule, its two confirmed instances, and how to scope the check are
+in **A hardening change must not turn a failing request into a succeeding one** in
+`.agents/skills/validate-integration/SKILL.md`.
+
+Note the matching asymmetry inside `safeUrlPath`: it rejects only a **truly empty** path component,
+never a whitespace-only one. Git tracks a file and a directory whose entire name is spaces, and the URL parser never removes `%20%20%20` the way it removes a dot segment — so
+rejecting it has no security value and breaks a legitimate path. `safeUrlPathSegment` still rejects an
+all-whitespace value, because it trims opaque ids first.
+
+### `params.x?.trim()` guards `undefined`, not the type
+
+A param's declared `type: 'string'` is enforced nowhere between the LLM tool call — or a
+`<Block.output>` reference resolved out of stored workflow state — and your URL builder. A
+numeric-looking id (a Vercel `deploymentId`, a Daytona `sandboxId`) arrives as a JSON **number** and
+stays one, so `params.id?.trim()` throws a bare `TypeError: params.id.trim is not a function` naming
+neither the tool nor the parameter.
+
+`toGuardedString` (`apps/sim/tools/url-path.ts:98`) is why the helpers do not have this problem: it
+accepts `string`, `number`, and `bigint`, rejects everything else **by parameter name**, and refuses
+number spellings whose decimal text is not the id the caller meant. Route path params through the
+helpers instead of hand-rolling an optional-chained `trim()`.
+
 ## Critical Rules for Outputs
 
 ### Output Types
@@ -527,6 +622,10 @@ All tool IDs MUST use `snake_case`: `{service}_{action}` (e.g., `x_create_tweet`
 - [ ] No tool declares `directExecution`; in-process work uses a registered operation
 - [ ] All params have explicit `required: true` or `required: false`
 - [ ] All params have appropriate `visibility`
+- [ ] No param is named `timeout`, `proxyUrl`, or `method` unless it means what the transport means
+- [ ] Every param interpolated into a request path goes through a `tools/url-path.ts` helper
+- [ ] No newly-guarded parameter on a destructive request turns a previously failing call into a
+      succeeding one — those use `strictUrlPathSegment`, not `safeUrlPathSegment`
 - [ ] All nullable response fields use `?? null`
 - [ ] All optional outputs have `optional: true`
 - [ ] No raw JSON dumps in outputs
diff --git a/.agents/skills/validate-integration/SKILL.md b/.agents/skills/validate-integration/SKILL.md
index a009e51d1ac..621d961ece0 100644
--- a/.agents/skills/validate-integration/SKILL.md
+++ b/.agents/skills/validate-integration/SKILL.md
@@ -82,6 +82,10 @@ For **every** tool file, check:
   - `'user-only'` — for API keys, credentials, and account-specific IDs the user must provide
   - `'user-or-llm'` — for everything else (search queries, content, filters, IDs that could come from other blocks)
 - [ ] Every param has a `description` that explains what it does
+- [ ] No param is named `timeout`, `proxyUrl`, or `method` unless it means exactly what the shared
+      transport means — `apps/sim/tools/request-transport.ts` reads all three off `params`
+      (`:191`, `:198`, `:167`), and `timeout` is milliseconds. See **Reserved Parameter Names** in
+      `.agents/skills/add-tools/SKILL.md`
 
 ### Request
 - [ ] URL matches the API endpoint exactly (correct base URL, path segments, path params)
@@ -92,8 +96,15 @@ For **every** tool file, check:
 - [ ] `Content-Type` header is set for POST/PUT/PATCH requests
 - [ ] Body sends all required fields and only includes optional fields when provided
 - [ ] For GET requests with query params: URL is constructed correctly with query string
-- [ ] ID fields in URL paths are `.trim()`-ed to prevent copy-paste whitespace errors
-- [ ] Path params use template literals correctly: `` `https://api.service.com/v1/${params.id.trim()}` ``
+- [ ] Every param interpolated into a URL path goes through a helper from `apps/sim/tools/url-path.ts`
+      (`safeUrlPathSegment` for an opaque id, `safeUrlPath` for a documented slash-delimited path,
+      `safeEncodedUrlPathSegment` for a single value that may contain `/`) — never a bare
+      `` `${params.id.trim()}` `` and never a bare `encodeURIComponent`. See **Path Parameters:
+      Reject Traversal, Never Just Encode** in `.agents/skills/add-tools/SKILL.md` for why encoding
+      is insufficient and why `params.id?.trim()` throws a raw `TypeError` on a numeric id.
+      Only `safeUrlPathSegment` is on `staging` — the others arrive with the path-safety sweep, so
+      check what your checkout exports and add a missing helper to `url-path.ts` rather than
+      hand-rolling one at the call site
 
 ### Response / transformResponse
 - [ ] Correctly parses the API response (`await response.json()`)
@@ -329,13 +340,265 @@ If any tool lists, searches, exports, imports, downloads, uploads, paginates, ba
 - [ ] Large result payloads are summarized, paginated, referenced, or capped rather than raw-dumped
 - [ ] Pagination and download tests cover caps, early stop behavior, or partial-result preservation when relevant
 
-## Step 9: Validate Error Handling
+## Step 9: Validate Path-Traversal Safety
+
+If any tool interpolates a param into a URL path, the integration needs a path-safety suite. Design it
+as follows — the naive shape does not work, and looks like it does.
+
+**Where these references live.** The shared harness (`apps/sim/tools/__tests__/path-safety.ts`), the
+per-service `path_safety.test.ts` suites, and every `url-path.ts` helper except `safeUrlPathSegment`
+arrive with the path-safety sweep. References below name **module and symbol** rather than a line,
+because those files are still being rebased and any line number would be stale on arrival.
+
+**Why the naive harness is worthless.** A suite that fuzzes every param at once and wraps the build in
+`try { ... } catch { return }` reports green on completely unguarded parameters: the first *guarded*
+sibling throws, the case is skipped, and every unguarded sibling is never exercised. Whole tools drop
+out of coverage the same way, and an aggregate count cannot detect it, because a case that never
+existed cannot fail. `x_manage_block.targetUserId` and `okta_remove_user_from_group.userId` were both
+fully unguarded while passing every vector a suite of that shape threw at them.
+
+A sound suite has five properties:
+
+1. **Enumerate (tool, param) pairs, fuzz one at a time.** Discover pairs by probing — build the URL
+   with a sentinel in one param and check whether it lands in `pathname` — and hold every sibling at a
+   safe value. Give *other* number params a real number so a sibling's own validation cannot abort the
+   build. Reference: `discoverPathParams` in `apps/sim/tools/__tests__/path-safety.ts`.
+2. **Assert rejection, not path shape.** A shape assertion (origin + segment count + unchanged
+   segments) catches only a minority of the vectors: `%2F` is never decoded back into a separator, and
+   a trailing bare `.` collapses to the parent with the segment count intact. Add an explicit
+   assertion that the build throws **and that the message names the parameter** — matched on whole
+   tokens, never `new RegExp(paramName)`, which is the substring trap recorded below (`"projectId
+   cannot have leading whitespace"` would satisfy a check for `id`). Capture the message and assert
+   it; do not use `toThrow(string)`, which is also a substring match:
+
+   ```typescript
+   let message = ''
+   try {
+     build(param, '..')
+   } catch (error) {
+     message = getErrorMessage(error, 'unknown error')
+   }
+   expect(message, `${param} accepted ".."`).not.toBe('')
+   expect(namesParam(message, param), `error did not name ${param}: ${message}`).toBe(true)
+   ``` Reference: the rejects-by-name assertions in `itResistsTraversal`.
+3. **Probe conditional branches.** A param that only appears on one branch of a conditional URL builder
+   is invisible to a single all-params probe. Harvest the string literals the builder compares against
+   and re-probe under each — in **pairs** as well as singly, since a param reachable only when two
+   siblings hold specific values is invisible to one-at-a-time probing — **and** probe the presence
+   branches, the shape taken when an optional param is *absent*.
+4. **Assert the skipped and unbuildable sets are empty.** Record every failed baseline build with its
+   reason and assert the ledger against an explicit, justified allowlist, plus a second assertion that
+   no URL-building tool sits outside the suite unaccounted for. `discoverPathParams` returns
+   `unbuildable`, `undiscoverable`, and `withoutPathParams` from a single sweep for exactly this. This is the check that surfaced a
+   tool missing from an entire suite.
+5. **Pin the legitimate values too.** `..foo`, `foo..`, `v1.2.3`, and a UUID must pass through
+   unaltered — a guard that over-rejects is its own bug.
+
+### Never let a `catch` stand in for an assertion
+
+**A tolerated throw must be tolerated by name**, in an explicit allowlist, with the reason recorded.
+A blanket `catch { return }` converts every case it covers from *tolerated* to *untested*.
+
+Keep the line that makes this usable: tolerating a failed **probe** during discovery is legitimate —
+probing a guarded param is *meant* to throw. Tolerating a throw inside an **assertion** is the bug.
+
+This was by far the most-repeated defect of the hardening effort — **nine** recorded instances, each
+an assertion that could not fail. The shapes are worth knowing individually, because only the first
+two look like a `catch`:
+
+| Shape | What it hid |
+|---|---|
+| A sibling-masking swallow in the traversal assertion | A fully unguarded parameter stayed green — reverting `x_manage_block.targetUserId` changed nothing |
+| A coverage pin enumerated through a function-only narrowing | 11 tools across four services were invisible, so `toEqual([...])` passed *because* they could not be seen |
+| The `preservesWhitespace` branch swallowing a rejection | The branch whose docstring says padding must survive to the wire tolerated a refusal of it |
+| The renders-inert cases swallowing any throw | Over-tightening — see below |
+| A **substring** matcher behind "the error names the parameter" | `"projectId cannot have leading whitespace"` satisfied the assertion for param `id`, and `"pathological failure"` satisfied `path` — so a guard naming the **wrong** identifier passed the very check meant to catch that. Match whole tokens (or a run of adjacent tokens, so `"Invalid function name"` still names `functionName`) |
+| An assertion that **guarded itself out of existence** | `if (serialized?.includes('projectId')) { expect(...) }` stopped verifying the moment a body dropped the field. Assert unconditionally |
+| `describe.each` over a silently-empty derived array | The group was filtered out of existence by a rename; a floor on the *total* still passed, so a whole block of assertions vanished emitting neither tests nor failures. Pin each derived group, not their sum |
+| **Fixture drift** — an assertion outliving the input it was written against | `expect(serialized).not.toContain('  my-project  ')` was written when the fixture *was* padded. A strict guard made padding throw, the fixture was unpadded, and the assertion stayed — now asserting the absence of a string that can no longer occur. It passes forever |
+| **A globally-mocked dependency** the assertion reads through | `vitest.setup.ts:112` stubs `@/tools/registry` as `{ tools: {} }`, so a guard that iterates the registry sees nothing and passes over an empty set. Four real failures only reproduced once `vi.unmock('@/tools/registry')` was added (`apps/sim/blocks/blocks/github.test.ts:3`) |
+
+Note that the last two have **different causes** from the rest, which is why auditing `catch` blocks
+and filters does not find them. The first four are *a tolerance applied too broadly*; fixture drift is
+*an assertion outliving its input*; the mock is *the assertion never reaching the real subject*. Check
+all three.
+
+**When an assertion couples two literals, derive one from the other instead of re-pinning it.** That
+is the fix that actually removes fixture drift rather than resetting its clock — a test named "agrees"
+should assert agreement, not two constants that happen to match:
+
+```typescript
+const urlProject = url.pathname.split('/projects/')[1]?.split('/')[0]
+expect(serialized).toContain(`"projectId":"${urlProject}"`)
+```
+
+A matcher is the highest-leverage instance of this: a weakness in it is invisible from every suite it
+powers, so give a shared matcher its own contract test (`namesParam` in
+`apps/sim/tools/__tests__/path-safety.ts` has one) and verify it red by reverting to the loose form.
+
+**The last one fails in the opposite direction from everything else here, and that is the point.**
+`catch { return }` meant the origin check, the prefix check, and the inert-probe assertion never ran —
+so a guard that **over-tightened**, rejecting a value it should merely have encoded, passed silently.
+A path-safety suite that only catches under-guarding is half a suite.
+
+The resolution is the shape to copy:
+
+1. Enumerate every (tool, param) pair against every inert value.
+2. **Measure** which ones legitimately throw, and establish why — do not assume.
+3. Make a throw a **failure** unless the parameter is named in an explicit allowlist
+   (the `strictlyValidated` option), with the justification in its TSDoc.
+
+Measured on that PR, exactly ten pairs legitimately reject: Supabase `table` via
+`validateDatabaseIdentifier` and `functionName` via `validateFunctionName`
+(the Supabase suite passes them as `strictlyValidated`), correct because `abc#fragment` is a fine URL
+segment but not a SQL identifier. Every other pair must build.
+
+- [ ] A `path_safety.test.ts` exists for the service and enumerates (tool, param) pairs
+- [ ] Each pair asserts a *named* rejection of the bare `.` and `..` segments
+- [ ] Conditional and presence branches of every conditional URL builder are probed
+- [ ] The skipped/unbuildable ledger is asserted empty against a justified allowlist
+- [ ] Legitimate dot-bearing values pass through unchanged
+- [ ] No assertion wraps its subject in a blanket `catch` — every tolerated throw is allowlisted by
+      name with a recorded reason, and over-tightening fails the suite as loudly as under-guarding
+- [ ] Shared matchers have their own contract test, verified red against the loose form
+- [ ] Assertions pin exact encoded output and exact error text, never a bare `toThrow()`
+- [ ] **Every new assertion was verified red before it was kept** — revert the fix, watch it fail
+- [ ] **When a fixture changed, every assertion that referenced its old value was re-verified red**
+- [ ] Assertions that read a globally-mocked module (`@/tools/registry`, `@/blocks/registry`, …)
+      `vi.unmock` it first, or they pass over the stub
+
+### A hardening change must not turn a failing request into a succeeding one
+
+State it in exactly those words, and apply it to every guard you add. It is the check that caught a
+class of regression the whole sweep otherwise missed — sixteen PRs' own suites, both review bots on
+several passes, and the author.
+
+Adding a guard that **trims** a path identifier looks strictly safer. It is not, when that identifier
+reaches a destructive endpoint, because trimming is only neutral if the untrimmed value already
+worked. Where the parameter previously went out through a bare `encodeURIComponent`, it did not:
+
+```
+box_sign_cancel_request  (apps/sim/tools/box_sign/cancel_request.ts)
+  before: /2.0/sign_requests/%20%20<uuid>%20%20/cancel  -> 404, no-op
+  after:  /2.0/sign_requests/<uuid>/cancel              -> cancels a real signature request
+
+cloudflare_delete_r2_bucket  (apps/sim/tools/cloudflare/delete_r2_bucket.ts)
+  before: "  prod-data  " names no bucket that can exist -> request FAILS
+  after:  trimmed to "prod-data"                         -> DESTROYS the real bucket
+
+google_bigquery_delete_dataset / _delete_table  — same shape on projectId
+```
+
+**Reason about the intersection, not the guard's contract.** "Trimming is the helper's contract at all
+137 call sites" is an *average*, and it excuses the deletion above. The real question is a set
+intersection: which parameters does this change *newly* normalise, **and** which of those sit on an
+irreversible request (DELETE, cancel, revoke, purge, drop)? On the Cloudflare PR the answer was
+exactly **one of 137** — and it took four review passes escalating P2→P2→P1→P1, with two pushbacks,
+to establish it.
+
+**The resolution: refuse the padded value on those parameters.** Not on consistency grounds — on two
+facts specific to the values themselves:
+
+1. No legitimate identifier for these providers carries surrounding whitespace (a Box Sign id is a
+   UUID; a GCP project id matches `[a-z][a-z0-9-]{5,29}`; an R2 bucket name is
+   `^[a-z0-9][a-z0-9-]*[a-z0-9]`), so refusing excludes nothing a caller could really mean.
+2. The previous behaviour was already a clean failure, so refusing preserves it — and improves on it
+   by replacing an opaque provider 404 with an error naming the parameter.
+
+`strictUrlPathSegment` in `apps/sim/tools/url-path.ts` is `safeUrlPathSegment` with that precondition,
+and `strictEncodedUrlPathSegment` is its `%2F`-preserving counterpart. Derive any body copy of the same
+identifier from the same guard, so the path value and the body value share one rule rather than two.
+
+Parameters that were **already** trimmed before your change keep plain `safeUrlPathSegment` — that is
+not a change you are making, and tightening them would break callers whose stored value works today.
+
+- [ ] Enumerated the parameters whose normalisation this change actually alters
+- [ ] Intersected that set with destructive operations and checked each member individually
+- [ ] Newly-trimmed identifiers on irreversible requests refuse the padded value rather than trimming it
+- [ ] Already-trimmed identifiers were left alone
+
+### Your tests can lie to you
+
+Two ways a green suite means nothing, both hit during this effort.
+
+#### Assert exact error text and exact encoded output — never a bare `toThrow()`
+
+This is the **counterweight** to the rule above. "Never let a `catch` stand in for an assertion" says
+*make sure it can fail*; this one says *make sure it fails for the right reason*.
+
+A guard your suite exercises usually lives in a module someone else owns, so its behaviour changes
+land underneath the suite without touching a line of it. Pinning exact strings is what converts that
+into a failing test instead of a silent behaviour change. On this effort it caught three separate
+upstream changes to `url-path.ts` that a `toThrow()`-only suite goes green through:
+
+- `safeUrlPath` stopped trimming each segment, so a storage key's interior whitespace began surviving
+  to the wire — caught by an **equality assertion on the encoded output**.
+- Its empty-segment check narrowed from `!segment.trim()` to `!segment`, so `a/ /b` became legal while
+  `a//b` stayed rejected — caught by an assertion on the **exact error text**. This one is a silent
+  correctness change in *either* direction: permitting `a//b` retargets the request at a different
+  object, rejecting `a/ /b` makes a real object key permanently unreachable.
+- A rebase changed a guard's wording from `"cannot have"` to `"must not have leading or trailing
+  whitespace"`, failing four assertions. They were **updated to the new wording, not loosened** — that
+  precision is the property that caught the other two.
+
+**`toThrow('...')` does not do this.** Vitest treats a string argument as a *substring* match, so it
+is only a slightly tighter `toThrow()` and drifts for the same reason. Capture the message and assert
+equality, and pin the output with `toBe`:
+
+```typescript
+expect(decodeURIComponent(url.pathname)).toBe('<the exact expected path>')
+
+let message = ''
+try {
+  build(...)
+} catch (error) {
+  message = getErrorMessage(error, 'unknown error')
+}
+expect(message).toBe('<the exact message>')
+```
+
+Never relax one of these to `toThrow()`, `toThrow(string)`, or `toContain()` to make a rebase quiet —
+update the expectation to the new wording instead.
+
+#### These tests are type-checked by nothing
+
+`apps/sim/tsconfig.json` excludes `**/*.test.ts` and `**/*.test.tsx` from `include`, and
+`apps/sim/vitest.config.ts` declares no `typecheck` block — Vitest transpiles with esbuild and never
+type-checks. So `bun run type-check` will pass over a harness full of type errors, and a harness that
+silently narrows to `any` will still run.
+
+To type-check one, write a temporary tsconfig that extends `apps/sim/tsconfig.json`, drops the
+`**/*.test.ts` exclusion, and includes only the harness — then delete it. Do not commit it; the
+exclusion exists deliberately.
+
+#### A test that calls a function directly can pass while the wrapper does the opposite
+
+`executeTool` wraps every `postProcess` call in a catch that logs and then restores the
+pre-`postProcess` result (`apps/sim/tools/index.ts:1977` and `:2062`). For a submit-then-poll tool
+that pre-`postProcess` result is the **submit** response — `success: true` with every result field
+null. So a `postProcess` that throws on a timed-out or exhausted poll is reported to the user as a
+successful lookup that simply found nothing — and that stale success is also what reaches the
+hosted-key cost hook (`:1987`), which runs on `finalResult.success`.
+
+Whether the caller is then charged for it depends entirely on the tool's own `getCost`. Enrow's
+returns 0 when the output carries no `qualification`, precisely because the fall-back submit response
+has none (`apps/sim/tools/enrow/verify_email.ts:45`, `find_email.ts:56`) — so it does not bill, by
+deliberate design. A tool priced per request rather than per qualified result would. **Write
+`getCost` so it cannot charge for a result the poll never produced**, and do not rely on the failure
+propagating: it does not.
+
+Eleven Enrow failure-path tests asserted that throwing contract by calling `postProcess` directly.
+Every one passed. Production reported `success: true`. Assert through the real call path — or, where
+a test must call the function directly, assert the shape the *executor* will hand back, not the one
+the function raises.
+
+## Step 10: Validate Error Handling
 
 - [ ] `transformResponse` checks for error conditions before accessing data
 - [ ] Error responses include meaningful messages (not just generic "failed")
 - [ ] HTTP error status codes are handled (check `response.ok` or status codes)
 
-## Step 10: Report and Fix
+## Step 11: Report and Fix
 
 ### Report Format
 
@@ -446,6 +709,14 @@ After fixing, confirm:
 - [ ] Regenerated deployment config when block/OAuth metadata changed and ran both catalog checks
 - [ ] Validated pagination consistency across tools and block
 - [ ] Validated memory load safety using `.agents/skills/memory-load-check/SKILL.md` when tools list/search/download/import/export/batch data
+- [ ] Validated path-traversal safety: url-path helpers at every path interpolation, and a
+      `path_safety.test.ts` that enumerates (tool, param) pairs, asserts named rejection, probes
+      conditional and presence branches, and asserts an empty skip ledger
+- [ ] Validated no param collides with a transport-reserved name (`timeout`, `proxyUrl`, `method`)
+- [ ] Confirmed no hardening change turns a failing request into a succeeding one — newly-normalised
+      parameters intersected with destructive operations, each checked individually
+- [ ] Confirmed failure-path tests assert through the real call path, not a direct call the executor
+      wraps differently
 - [ ] Validated error handling (error checks, meaningful messages)
 - [ ] Validated registry entries (tools and block, alphabetical, correct imports)
 - [ ] Validated model-visible/opaque inputs and Sim-durable/internal-execution provenance at their