Skip to content

fix: reject dot-segment path parameters; disable Speakeasy persistent edits - #1372

Merged
alexb-openrouter merged 6 commits into
mainfrom
chore/remove-call-model
Nov 13, 2026
Merged

alexb-openrouter merged 6 commits into
mainfrom
chore/remove-call-model

Conversation

@mattapperson

@mattapperson mattapperson commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two things, both patch-level:

  1. Turns off Speakeasy persistent edits, and moves the hand edits that depended on it out of generated code.
  2. Security fix: a path parameter of exactly .. or . no longer sends the request to a different route. Supersedes fix(url): reject "." and ".." path parameters so URL resolution can't change the route #1390, whose fix was a hand edit to a generated file and would be overwritten once persistent edits are off.

No version change and no API removal: this is intended to ship as an ordinary patch release.

The branch is up to date with main (8d929a22, SDK 1.5.102). The merge takes main's generated src/lib/env.ts (the OPENROUTER_DEBUG parser fix) without the OPENROUTER_BASE_URL hand edit, keeps src/lib/url.ts out of gen.lock trackedFiles, and strips @generated-id headers on files added since. See "Dropped from the original PR" at the bottom.

Security fix: dot-segment path parameters

Generated operations percent-encode path parameters, which leaves . and .. unchanged. ClientSDK._createRequest (src/lib/sdks.ts) then builds the URL with new URL(path, baseURL), which resolves them as dot segments. Code that passes an untrusted value as a path parameter could therefore have the request, with the client's API key, sent to another route:

client.containers.downloadContainerFileContent({ containerId: "..", fileId: "file-abc" })
// before: GET /api/v1/files/file-abc/content
// after:  rejects with Error "Parameter 'container_id' cannot be the dot segment '..'"; nothing is sent

Behaviour. pathToFunc (src/lib/url.ts), which every operation uses to fill its path template, now throws when a parameter that forms a whole path segment is exactly . or ... There is no whole-path check, so a base URL that itself contains a dot segment (serverURL or OPENROUTER_BASE_URL) normalises as on main. ..., .env, a..b are unaffected, %2e%2e is still sent double-encoded as %252e%252e, and the existing "is required" / "must be a string or number" errors are unchanged. The only inputs that behave differently are ones that were already being misrouted.

Error type. A plain Error, as a rejected promise, the same as the existing "Parameter 'x' is required" error. It is thrown before _createRequest, so it is not wrapped in the SDK's Result / InvalidRequestError, including for callers of the standalone funcs/* functions.

Mechanism: src/lib/url.ts is added to .genignore and is now hand-maintained. The alternatives were tried first:

Option Result
OpenAPI pattern on path-parameter schemas via an overlay Tested with the pinned generator: an overlay put the pattern on all 111 path-parameter schemas in out.openapi.yaml, and regeneration produced no change under src/. The generator does not emit pattern (or minLength: the spec has both today and the generated zod schemas have neither).
gen.yaml option None exists for path-parameter encoding or validation.
SDK hook Not sound. beforeCreateRequest runs at the end of _createRequest, after new URL(path, baseURL), and receives only the resolved URL; HookContext has no path template or raw parameters. A misrouted /files/file-abc/content is indistinguishable from a real call to that route.

Regenerating with the entry in place leaves src/lib/url.ts byte-identical, and the generator removes the file's entry from gen.lock trackedFiles, which is committed here.

Cost and mitigation. Generator updates to this one file no longer arrive. tests/unit/path-param-dot-segments.test.ts has a "generator contract" block that fails if generated operations start using the file differently (the exported signature, what they import from it, the call shape).

Removing it. The bug has been reported to Speakeasy. When the generator rejects or safely encodes dot segments: delete the src/lib/url.ts line from .genignore, regenerate, keep the behaviour tests. The file's header comment says the same.

Consumer-visible behaviour changes

Please read these before deciding this is fine as a patch.

  1. serverURLFromOptions() called directly no longer reads OPENROUTER_BASE_URL. With the env var set, serverURLFromOptions({}) now returns the production URL. new OpenRouter() and every sub-SDK (chat, etc.) still honour the env var with the same precedence, through a hook. This one is not avoidable while persistent edits are off: the helper lives in generated src/lib/config.ts, and Speakeasy's TypeScript target has no setting that makes the server URL read an env var (envVarPrefix covers security and global parameters only). Workaround for direct callers: serverURLFromOptions({ serverURL: process.env.OPENROUTER_BASE_URL }).
  2. Env and envSchema in src/lib/env.ts no longer have an OPENROUTER_BASE_URL field. Only code that deep-imports @openrouter/sdk/lib/env.js and reads env().OPENROUTER_BASE_URL is affected. Same cause as (1).
  3. Minor, strictly more permissive: the hook reads OPENROUTER_BASE_URL each time a client is constructed. Before, the value was memoised with the rest of env() on first use. client._options.serverURL is now populated with the env value when it is applied.

Not changed:

  • Precedence is still serverURL > server > OPENROUTER_BASE_URL > production.
  • An invalid OPENROUTER_BASE_URL still throws a ZodError (issue path ["OPENROUTER_BASE_URL"]) at client construction, including when serverURL is passed explicitly. The original version of this PR changed it to a TypeError; that is gone.
  • The // @generated-id comment line disappears from generated sources. Comment only, no runtime or type effect.

Audit of hand edits

Every non-deleted file under trackedFiles in .speakeasy/gen.lock (3,955 files) was hashed and compared with its pristine_git_object. Re-run in this round against main at a709c011: the same five differ, nothing new.

File Hand edit Where it went
src/lib/config.ts OPENROUTER_BASE_URL fallback in serverURLFromOptions (#832) SDKInitHook in src/hooks/base-url-env.ts, registered in src/hooks/registration.ts. Direct helper calls lose it (change 1 above).
src/lib/env.ts OPENROUTER_BASE_URL in Env / envSchema (#832) Same hook, which validates with the same zod rule (change 2 above).
RUNTIMES.md "v18, v20, and v22" Added to .genignore
.devcontainer/devcontainer.json Node 24 bookworm image (new since the original audit) Added to .genignore
tsconfig.json one whitespace-only line removed Back to generated form

src/sdk/sdk.ts no longer differs from pristine: the callModel regions were removed on main in #1356. enableCustomCodeRegions is a separate feature and stays on.

Existing hooks are unaffected: src/hooks/signal-hooks.ts and registration.ts are not generator-owned, and the regen did not touch them. The new hook is registered before SignalTimeoutHook; the two do not interact.

Commits

  1. chore: disable Speakeasy persistent edits: persistentEdits.enabled: never, .genignore, the hook and its registration, tests, CLAUDE.md, and the regenerated config.ts / env.ts / tsconfig.json. This is the one to review.
  2. chore: regenerate SDK without @generated-id headers: generator output only. 1,392 files lose the one header line, gen.lock drops the matching tracking data, and Speakeasy removes one blank line from .gitignore. Nothing else.
  3. test: remove unused schema-test-helpers: a test helper left behind by SEC-768: refactor: remove the old callModel code that moved to @openrouter/agent #1356 that nothing imports.
  4. fix(url): reject "." and ".." path segments so URL resolution can't change the route: src/lib/url.ts, its .genignore entry, the gen.lock entry removal, tests, CLAUDE.md.

Version

Left at 1.5.85 in gen.yaml, package.json, jsr.json and SDK_METADATA, exactly as on main. Versioning is versioningStrategy: automatic and the Generate workflows run sdk-generation-action in mode: pr, which bumps the patch when it regenerates. gen.lock here keeps the old docChecksum and has a new configChecksum, so the next release regen sees a change and bumps to the next patch.

How it was regenerated

Pinned Speakeasy 1.803.2, local only:

speakeasy run -t openrouter --skip-upload-spec --skip-versioning -o console -y

with the two registry: blocks removed from an uncommitted copy of workflow.yaml (code samples written to a local file), so nothing was uploaded or tagged. workflow.yaml and workflow.lock are unchanged from main.

main's in.openapi.yaml is one spec update (#1434) ahead of its generated code. To keep this diff to the persistent-edits change, generation used the spec revision the current generated code was built from. The pending spec changes are left for the normal Generate run.

A run on the final tree (after commit 4) produced no diff apart from key ordering inside package.json, which the generator shuffles on every run and which is not committed. src/lib/url.ts had the same hash before and after.

Testing

  • pnpm run lint, pnpm run typecheck, pnpm run build: pass.
  • pnpm test: 122/122 in 10 files, no type errors. tests/unit/path-param-dot-segments.test.ts (28 tests) runs three operations (containers.downloadContainerFileContent, batch.getBatches, presets.restore) against a local mock server and checks the route hit, that . / .. reject with the parameter named and send nothing, the unaffected values, numeric parameters, and the generator contract. 11 of them fail with main's url.ts. tests/unit/server-url-env.test.ts (10 tests) goes through new OpenRouter() and covers: unset, env applied, env applied to chat, serverURL over env, server over env, serverURL over server, invalid value is a ZodError with the right path, invalid value still throws with explicit serverURL, empty value throws, and the direct-helper behaviour.
  • End to end through built esm/ against a local HTTP stub: with OPENROUTER_BASE_URL set, chat.send hit the stub at the env path; an explicit serverURL won; server: "production" won; unset fell back to production.
  • End to end through esm/ built from the regenerated tree: containerId .. and ., batch.getBatches({ id: ".." }) and files.download({ fileId: "." }) threw and the stub received nothing; a normal id and %2e%2e hit the container route.
  • Not run: the e2e suite against the live API.

Dropped from the original PR

  • The v2.0.0 release: no version bump, no feat! / chore!, no "Migrating from v1" README section. The README on main already says callModel moved to @openrouter/agent.
  • The callModel removal commit: already on main via SEC-768: refactor: remove the old callModel code that moved to @openrouter/agent #1356. Of the 37 files that commit deleted, two are still on main: tests/utils/schema-test-helpers.ts (unreferenced, removed here) and src/models/claude-message.ts (kept: it is listed as hand-written in CLAUDE.md and is reachable as a public deep import, so deleting it would be a consumer-visible removal).
  • The TypeError for an invalid OPENROUTER_BASE_URL: reverted to the existing ZodError.

Notes

  • Once this merges, any hand edit to a generated file is overwritten by the next regen without warning. CLAUDE.md is updated to say so. src/lib/url.ts is the exception, because it is in .genignore.
  • The next Generate PR will conflict with this one on the header lines, whichever lands first; rerunning generation resolves it.

devin-ai-integration[bot]

This comment was marked as resolved.

@mattapperson
mattapperson force-pushed the chore/remove-call-model branch from e1b6c79 to 3f1d10b Compare November 5, 2026 17:26
mattapperson added a commit that referenced this pull request Nov 5, 2026
Addresses review feedback on #1372. With persistent edits off, the
OPENROUTER_BASE_URL fallback lives in an SDKInitHook, so only client
construction honors it. The generated serverURLFromOptions() helper
reads only the options it is given. Keeping the env fallback in that
helper would need a hand edit to generated config.ts, which a regen
would now silently overwrite.

Document this in the v2 migration notes, explain it in the hook's
comment, and add a test that pins the helper's behavior.
Set persistentEdits.enabled to "never" so generated files are plain
generator output and Speakeasy does not prompt to turn the feature back on.

Every Speakeasy-tracked file was compared against its pristine blob in
gen.lock. Five carried hand edits; each is handled without persistent edits:

- src/lib/config.ts, src/lib/env.ts: OPENROUTER_BASE_URL support (#832)
  moves to an SDKInitHook (src/hooks/base-url-env.ts) registered from the
  hand-written src/hooks/registration.ts. Precedence is unchanged
  (serverURL > server > OPENROUTER_BASE_URL > production) and an invalid
  value still throws a ZodError at client construction.
- RUNTIMES.md (Node v22 note) and .devcontainer/devcontainer.json (Node 24
  image): added to .genignore.
- tsconfig.json: a whitespace-only line, back to the generated form.

The exported serverURLFromOptions() helper is generated code and no longer
reads OPENROUTER_BASE_URL when called directly; clients built with
new OpenRouter() are unaffected. A unit test pins this.
Output of the pinned Speakeasy 1.803.2 with persistent edits off, generated
from the same spec revision as the current generated code. Removes the
`@generated-id` header line from 1,392 generated files and drops the
matching tracking data from gen.lock. No other source changes. The SDK
version is left for the release workflow to bump.
Left over from the callModel removal (#1356); nothing imports it.
@mattapperson
mattapperson force-pushed the chore/remove-call-model branch from fb77bcd to 6c531cc Compare November 11, 2026 17:41
@mattapperson mattapperson changed the title feat!: remove callModel (moved to @openrouter/agent), disable persistent edits, release v2.0.0 chore: disable Speakeasy persistent edits Nov 11, 2026
…hange the route

Generated operations percent-encode path parameters, which leaves "." and
".." unchanged, and ClientSDK._createRequest resolves the path with
new URL(path, baseURL), which collapses dot segments. A path parameter of
exactly ".." or "." sent the request, with the client's API key, to a
different route.

pathToFunc now throws before any request is built. src/lib/url.ts is added
to .genignore and hand-maintained, because persistent edits are off and
neither a hook, an overlay nor gen.yaml can express the check. gen.lock
drops the file's tracking entry, as the generator does for ignored files.

Supersedes #1390.
@mattapperson mattapperson changed the title chore: disable Speakeasy persistent edits fix: reject dot-segment path parameters; disable Speakeasy persistent edits Nov 11, 2026
Comment thread src/lib/url.ts Resolved

@christineschen christineschen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one small nit but otherwise lgtm

devin-ai-integration Bot and others added 2 commits November 13, 2026 19:59
Take main's env.ts and gen.lock, keep src/lib/url.ts out of the tracked
files, strip @generated-id headers that main reintroduced, and drop the
whole-path dot-segment backstop in pathToFunc so base URLs that contain
dot segments normalise as before.
The base-URL env var is read by the base-url-env hook on this branch;
the merge from main brought back the hand edit in src/lib/env.ts.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@alexb-openrouter
alexb-openrouter merged commit 509a060 into main Nov 13, 2026
2 checks passed
@alexb-openrouter
alexb-openrouter deleted the chore/remove-call-model branch November 13, 2026 20:06
alexb-openrouter added a commit that referenced this pull request Nov 13, 2026
## Summary

The last three Publish runs (1.5.101, 1.5.102, and the merge of #1372)
failed in `npm run build` with `FATAL ERROR: ... JavaScript heap out of
memory` in `tsc`, so nothing has reached npm since 1.5.100.

Cause: the Publish workflow runs `npm install` without a lockfile, so it
resolves the newest `zod` 4.x (4.6.7). `pnpm-lock.yaml`, used by PR
validation and local builds, pins 4.2.1. Type-checking the generated
models against zod 4.6.7 peaks above the default Node heap limit on the
runner (about 4 GB). With the lockfile, the build peaks at about 2.4 GB.

Change, in both `.speakeasy/gen.yaml` `additionalScripts` (source of
truth) and `package.json`:

```diff
- build:   tsc && node scripts/unbarrel-imports.js
+ build:   node --max-old-space-size=8192 ./node_modules/typescript/bin/tsc && node scripts/unbarrel-imports.js
  (same for compile)
```

`typecheck` is unchanged.

Verified on a fresh clone of this branch with `npm install
--ignore-scripts && npm run build`. It resolves zod 4.6.7 the same way
Publish does, and the build completes in 37s with a peak RSS of 4.5 GB.

This PR doesn't touch `.speakeasy/gen.lock`, so merging it doesn't
trigger Publish. After merging, run Publish manually
(`workflow_dispatch`) to release the current `main` version.

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants