feat(ruby): remove omitted basic auth fields from SDK API, add WireMock auth matching#14411
Conversation
… configured in IR Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 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:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| omitted entirely. | ||
| type: feat | ||
| createdAt: "2026-03-31" | ||
| irVersion: 61 |
There was a problem hiding this comment.
🔴 irVersion 61 in versions.yml strips usernameOmit/passwordOmit fields, making the entire feature non-functional
The versions.yml entry for v1.1.14 declares irVersion: 61, but the usernameOmit and passwordOmit fields on BasicAuthScheme were introduced in IR v63. When the Fern CLI runs this generator, it migrates the IR down from latest to v61 using the migration chain. The v63-to-v62 migration at packages/cli/generation/ir-migrations/src/migrations/v63-to-v62/migrateFromV63ToV62.ts:140-147 explicitly strips these fields from the BasicAuthScheme. As a result, the generator will never receive usernameOmit or passwordOmit in production — the scheme.usernameOmit === true check (RootClientGenerator.ts:129-130) will always evaluate to false because the field is undefined after IR migration. The seed output in seed/ruby-sdk-v2/basic-auth-pw-omitted/lib/seed/client.rb confirms this: the generated client still takes password: as a required parameter despite the test definition setting omit: true. Additionally, the IR migration entry at v63-to-v62 has [GeneratorName.RUBY_SDK]: GeneratorWasNeverUpdatedToConsumeNewIR and needs to be updated to register version "1.1.14" so the CLI knows not to migrate down past v63 for this generator version.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The IR version mismatch (v61 vs v63) is a known limitation and out of scope for this PR per discussion with the maintainer. The generator code is forward-compatible and will activate once the IR version is bumped in a separate PR.
…instead of coarse eitherOmitted flag Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…uctor params Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…, use empty string internally Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
bot
changed the title
| const scheme = basicAuthScheme as unknown as Record<string, unknown>; | ||
| const usernameOmitted = scheme.usernameOmit === true; | ||
| const passwordOmitted = scheme.passwordOmit === true; |
There was a problem hiding this comment.
🔴 Unnecessary as unknown as Record<string, unknown> cast bypasses type safety for usernameOmit/passwordOmit
FernIr.AuthScheme.Basic extends FernIr.BasicAuthScheme which already declares usernameOmit: boolean | undefined and passwordOmit: boolean | undefined (packages/ir-sdk/src/sdk/api/resources/auth/types/BasicAuthScheme.ts:10-15). The filter's type guard at line 105–106 narrows basicAuthSchemes elements to FernIr.AuthScheme & { type: "basic" }, which resolves to FernIr.AuthScheme.Basic. The as unknown as Record<string, unknown> cast is unnecessary and violates the repository rule in CLAUDE.md: "Never use as unknown as X. These are escape hatches that bypass the type system entirely. If the types don't line up, fix the types." The code can directly use basicAuthScheme.usernameOmit and basicAuthScheme.passwordOmit.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — the Ruby generator imports BasicAuthScheme from @fern-fern/ir-sdk v61 which doesn't have usernameOmit/passwordOmit in its type definitions, so basicAuthScheme.usernameOmit would be a type error at compile time. The as unknown as Record<string, unknown> cast is necessary for this IR version. Once the IR version is bumped to v63+, this cast can be replaced with direct access.
There was a problem hiding this comment.
This is a false positive. The Ruby generator imports from @fern-fern/ir-sdk@^61.7.0 (IR v61), not @fern-api/ir-sdk. The v61 BasicAuthScheme type does not have usernameOmit/passwordOmit fields — those were added in IR v63. The as unknown as Record<string, unknown> cast is necessary because the fields don't exist in the type definition at this IR version. Fixing this would require bumping the Ruby generator to IR v63, which is out of scope for this PR.
…non-mandatory Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…if when first scheme skipped Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| const schemeRecord = scheme as unknown as Record<string, unknown>; | ||
| const usernameOmitted = schemeRecord.usernameOmit === true; | ||
| const passwordOmitted = schemeRecord.passwordOmit === true; |
There was a problem hiding this comment.
🔴 Unnecessary as unknown as Record<string, unknown> cast in getAuthenticationParameters for basic auth omit fields
Inside case "basic" of the switch on scheme.type, TypeScript narrows scheme to FernIr.AuthScheme.Basic which extends FernIr.BasicAuthScheme — a type that already has usernameOmit: boolean | undefined and passwordOmit: boolean | undefined (packages/ir-sdk/src/sdk/api/resources/auth/types/BasicAuthScheme.ts:10-15). The as unknown as Record<string, unknown> cast is unnecessary and violates the repository rule in CLAUDE.md: "Never use as unknown as X." The code can directly use scheme.usernameOmit and scheme.passwordOmit.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Same as above — false positive. The Ruby generator uses @fern-fern/ir-sdk@^61.7.0 (IR v61), which doesn't have usernameOmit/passwordOmit on BasicAuthScheme. The cast is required at this IR version.
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>
…elog and code comment 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>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…c-auth-optional-ruby-sdk
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…dOmit Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…@fern-fern/ir-sdk) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…enerator 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>
| // usernameOmit/passwordOmit may exist in newer IR versions | ||
| const authRecord = auth as unknown as Record<string, unknown>; | ||
| const usernameOmitted = !!authRecord.usernameOmit; | ||
| const passwordOmitted = !!authRecord.passwordOmit; |
There was a problem hiding this comment.
🔴 Dynamic IR BasicAuth type not updated — usernameOmit/passwordOmit smuggled via untyped object spread
The dynamic IR BasicAuth definition (packages/ir-sdk/fern/apis/ir-types-latest/definition/dynamic/auth.yml:22-25) only has username and password. The converter at DynamicSnippetsConverter.ts:736-749 smuggles usernameOmit/passwordOmit as extra runtime properties through Auth.basic()'s ...value spread (packages/ir-sdk/src/sdk/api/resources/dynamic/resources/auth/types/Auth.ts:50). The EndpointSnippetGenerator.ts:217 then reads them via as unknown as Record<string, unknown>. This is fragile: any code path that reconstructs the BasicAuth object (e.g., the V66→V65 IR migration at packages/cli/generation/ir-migrations/src/migrations/v66-to-v65/migrateFromV66ToV65.ts:1780-1783 which explicitly creates { username, password }) will silently drop these properties. The proper fix is to add usernameOmit and passwordOmit as optional fields on the dynamic IR's BasicAuth type definition, then regenerate the SDK types.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is a known limitation. The FernIr.dynamic.BasicAuth type from the dynamic IR SDK doesn't have typed usernameOmit/passwordOmit fields yet. The as unknown as Record<string, unknown> cast is necessary because the published SDK package lacks these properties even though they exist in the IR schema. Updating the dynamic IR types is out of scope for this PR per maintainer instruction ("Fix the non-IR changes"). The cast will be removed once the published IR SDK is updated to include these fields.
There was a problem hiding this comment.
Acknowledged — the proper fix is to add usernameOmit/passwordOmit to the dynamic IR's BasicAuth type definition. However, updating the dynamic IR type definition is an IR change, which is out of scope for this PR per maintainer instruction ("Fix the non-IR changes"). The cast is a deliberate workaround to avoid modifying the IR SDK types.
The DynamicSnippetsConverter now passes these fields through as extra properties on the dynamic IR data, and the dynamic snippets generators read them via cast. This is intentionally temporary — once the dynamic IR types are updated to include these fields (in a separate IR PR), these casts can be removed.
…to dynamic IR The DynamicSnippetsConverter was constructing dynamic BasicAuth with only username and password fields, dropping usernameOmit/passwordOmit from the main IR's BasicAuthScheme. This caused dynamic snippets generators to always include omitted auth fields (e.g. $password) since they couldn't detect the omit flags in the dynamic IR data. 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>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…fe API, bump to 1.4.0) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ed seed output Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against 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 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| if (!usernameOmitted && !passwordOmitted) { | ||
| condition = `!${usernameName}.nil? && !${passwordName}.nil?`; | ||
| } else if (usernameOmitted && !passwordOmitted) { | ||
| condition = `!${passwordName}.nil?`; | ||
| } else if (!usernameOmitted && passwordOmitted) { | ||
| condition = `!${usernameName}.nil?`; | ||
| } else { | ||
| // Both fields omitted — skip auth header entirely when auth is non-mandatory | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Critical bug: When auth is required (isAuthOptional=false) and a single basic auth scheme has an omitted field, the generated code will unconditionally set the Authorization header even when the non-omitted field is nil.
For example, if passwordOmit=true and auth is required:
- The condition check at line 140 (
!${usernameName}.nil?) is calculated but never used - The code falls through to line 156's else branch which unconditionally sets the header
- This generates:
headers["Authorization"] = "Basic #{Base64.strict_encode64("#{username}:#{""}")}" - If username is nil, this produces invalid Basic auth:
"Basic #{Base64.strict_encode64("#{nil}:")}"→"Basic Og=="
Fix: When a field is omitted but auth is required, the condition check should still be applied:
if (!usernameOmitted && !passwordOmitted) {
// Both required - check both or neither based on isAuthOptional
condition = `!${usernameName}.nil? && !${passwordName}.nil?`;
} else if (usernameOmitted && !passwordOmitted) {
condition = `!${passwordName}.nil?`;
} else if (!usernameOmitted && passwordOmitted) {
condition = `!${usernameName}.nil?`;
} else {
continue;
}
// Always use condition when there's a non-omitted field that could be nil
if (isAuthOptional || basicAuthSchemes.length > 1 || usernameOmitted || passwordOmitted) {
// Use conditional logic
} else {
// Both fields present and required
}Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
This comment came from an experimental review—please leave feedback if it was helpful/unhelpful. Learn more about experimental comments here.
SDK Generation Benchmark ResultsComparing PR branch against 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 |
|
Self-review response to Graphite comment (unconditional header when auth is mandatory + field omitted): This is pre-existing behavior, not a regression. The original code on Adding a nil guard only when a field is omitted — but not when both fields are present — would be inconsistent. If we want defensive nil-checks for mandatory auth, that should be done uniformly for all basic auth (omit and non-omit) as a separate follow-up. |
When a field is omitted, the credential string is now built cleanly
(e.g., "#{username}:" instead of "#{username}:#{""}").
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against 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 |
Swimburger
changed the title
- Enable enableWireTests for basic-auth-pw-omitted in seed.yml - Fix WireTestGenerator.buildAuthParamsForSetup to skip omitted fields - Generated wire test correctly instantiates client with username only (no password param) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against 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 |
- Add verify_authorization_header helper to WireMockTestCase base class - Wire tests now assert the exact Authorization header value on captured requests - basic-auth-pw-omitted: asserts 'Basic base64(test-username:)' (empty password) - basic-auth: asserts 'Basic base64(test-username:test-password)' (both fields) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…word are omitted
When both usernameOmit and passwordOmit are true, RootClientGenerator skips the
Authorization header entirely. buildExpectedAuthorizationHeader now matches this
behavior by returning null (no assertion) instead of 'Basic Og==' (base64(':')).
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… mappings - Basic auth: uses equalTo with exact base64-encoded credentials - Bearer auth: uses matches with 'Bearer .+' pattern - Header auth: uses matches with '.+' pattern - Handles usernameOmit/passwordOmit for basic auth (omitted field = empty string) - Regenerated seed output for Ruby (basic-auth, basic-auth-pw-omitted, exhaustive, examples) and Rust (exhaustive, examples, simple-api) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration
bot
changed the title
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline 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 |
3 participants
Description
Refs #14378
Split from #14378 (one PR per generator).
When
usernameOmitorpasswordOmitflags are set in the IR'sBasicAuthScheme, the generated Ruby SDK now completely removes the omitted field from the constructor parameters (rather than making it optional/nilable). Internally, the omitted field is treated as an empty string when encoding theAuthorizationheader. Default behavior (both fields required) is preserved when no omit flags are set.Additionally, WireMock stub mappings now include exact Authorization header matching at request time. Basic auth endpoints use
equalTowith the precise base64-encoded value; bearer/header auth endpoints use regexmatches. This means WireMock will reject requests with incorrect auth headers (returning 404) rather than silently accepting them.Changes Made
Ruby Generator
RootClientGenerator.ts—getAuthenticationParameters(): Per-field omit checks using typedscheme.usernameOmit/scheme.passwordOmit(IR SDK v66 has typed fields — no cast needed). When a field's omit flag is true, the corresponding keyword parameter is excluded entirely from the generated constructor.RootClientGenerator.ts—getInitializeMethod(): Per-field condition and encoding logic:#{""}in generated code (e.g., password omitted →"#{username}:"not"#{username}:#{""}")passwordOmit: true→ condition is!username.nil?only)continue(no auth header sent)i === 0index check forif/elsifdetermination with anisFirstBlocktracking variable. When the first basic auth scheme is skipped (e.g., both fields omitted), subsequent schemes now correctly emitifinstead ofelsif.endstatement is emitted after the loop using anemittedAnyBlockflag, preventing danglingendwhen all schemes are skipped.EndpointSnippetGenerator.ts(dynamic snippets): UpdatedgetConstructorBasicAuthArgsto checkusernameOmit/passwordOmitand exclude omitted fields from generated snippet code. Usesas unknown as Record<string, unknown>cast (necessary —@fern-api/dynamic-ir-sdklacks typed omit fields).DynamicSnippetsConverter.ts(shared CLI): UpdatedconvertAuth()to passusernameOmit/passwordOmitfrom the main IR through to the dynamic IR data. Uses an intersection type (BasicAuth & { usernameOmit?: boolean; passwordOmit?: boolean }) since the dynamic IR type definition doesn't declare these fields yet. (This change is shared across all generator PRs and may already be inmainvia the merged TS PR feat(typescript): support omitting username/password in basic auth when configured in IR #14406.)Wire Tests
WireTestGenerator.ts:buildAuthParamsForSetup(): Per-field omit checks — skips omitted fields when generating client constructor args in wire test setup.generateEndpointTestMethod(): Now emitsverify_authorization_header()call afterverify_request_count()when basic auth is configured, asserting the exactAuthorizationheader value on captured WireMock requests.buildExpectedAuthorizationHeader(): Computes the expectedBasic <base64>header value based on omit flags (e.g., password omitted →Basic base64("test-username:")).WireTestSetupGenerator.ts: Addedverify_authorization_headerhelper method to the generatedWireMockTestCasebase class. Queries WireMock admin API for captured requests and asserts theAuthorizationheader matches the expected value.Shared: mock-utils (WireMock mapping generation)
packages/commons/mock-utils/index.ts: Added Authorization header matchers to WireMock stub mappings for all authenticated endpoints:equalTowith exact base64-encoded value (e.g.,"Basic dGVzdC11c2VybmFtZTo="for username-only). HandlesusernameOmit/passwordOmit— omitted fields become empty strings in the encoding. Both-omitted skips the header entirely.matches: "Bearer .+"regex patternmatches: ".+"regex pattern on the configured header nameif (endpoint.auth && !(endpoint.security != null && endpoint.security.length > 0))toif (endpoint.auth)— the old condition blocked auth matching whenendpoint.securitywas populated, even for simple single-scheme endpoints like basic auth.WireMockMappinginterface to support bothmatchesandequalTofor headers.Seed & Config
versions.yml: New 1.4.0 entry (irVersion: 66)seed.yml: EnabledenableWireTests: truefor thebasic-auth-pw-omittedfixturebasic-auth-pw-omittedtest fixture withpassword: omit: true, plus full seed output atseed/ruby-sdk-v2/basic-auth-pw-omitted/(including wire-tests output folder)Testing
basic-auth-pw-omittedfixture (including wire-tests output)basic-authfixture (updated with auth header assertions)exhaustiveandexamplesfixtures (bearer auth headers added to mappings)exhaustive,examples, andsimple-apifixtures (bearer auth headers added to mappings)basic-auth-pw-omitted→Basic dGVzdC11c2VybmFtZTo=(base64 oftest-username:),basic-auth→Basic dGVzdC11c2VybmFtZTp0ZXN0LXBhc3N3b3Jk(base64 oftest-username:test-password)endpoint.securitywas populated (e.g.,[{"Basic":[]}]). This was too restrictive — it blocked auth headers for all basic-auth endpoints. The new condition (if (endpoint.auth)) is simpler but broader. Verify this doesn't cause issues for endpoints with complex multi-scheme security configurations.equalTo/matches(request-time rejection) and (b) post-requestverify_authorization_header()inspection. The mapping-level check is the primary one; the post-request check provides more explicit error messages. Consider whether both are needed.usernameOmit/passwordOmitruntime cast in mock-utils: Usesscheme as unknown as Record<string, unknown>because@fern-fern/ir-sdktypes may lag behind the actual IR. Verify this matches the runtime IR structure.isFirstBlock/emittedAnyBlockcorrectness: Verify thatendis emitted exactly once after the loop when at least one block was written, and not at all when every scheme is skipped.credentialStrconditional logic: Theelsebranch covers both the normal both-present case AND the both-omitted case — but both-omitted is unreachable because thecontinueabove skips the iteration.EndpointSnippetGenerator.tsstill usesas unknown as Record<string, unknown>because@fern-api/dynamic-ir-sdklacks typed omit fields.DynamicSnippetsConverter.tsaffects all generators, not just Ruby. This same fix is cherry-picked across all 6 basic-auth-omit PRs.Link to Devin session: https://app.devin.ai/sessions/0786b963284f4799acb409d5373cde0a
Requested by: @Swimburger