Skip to content

test: cover optional security fixture - #3

Open
kriptoburak wants to merge 1 commit into
AVSystem:mainfrom
kriptoburak:codex/optional-security-fixture
Open

test: cover optional security fixture#3
kriptoburak wants to merge 1 commit into
AVSystem:mainfrom
kriptoburak:codex/optional-security-fixture

Conversation

@kriptoburak

Copy link
Copy Markdown

Summary

  • Extends the existing security schemes fixture with an anonymous {} security alternative.
  • Keeps the generated snapshot behavior pinned for specs that accept either public or API-key access.

Validation

  • pnpm install --ignore-scripts --frozen-lockfile
  • pnpm run build:debug
  • pnpm exec ava __test__/generate.snapshot.spec.ts --match='generate preserves full success payload snapshot for security-schemes.openapi.yaml'
  • git diff --check

Duplicate gate

  • Checked memory, target tree, direct PR and issue lists, and search until GitHub Search returned a rate-limit response. No same-surface duplicate found.

@pkurcx pkurcx self-assigned this Aug 20, 2026
@pkurcx

pkurcx commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Code review

No correctness bugs; safe to merge. Found 3 issues, all low severity:

  1. The added - {} exercises no code path. security is never deserialized: the Operation struct has no security field and is not deny_unknown_fields, so serde discards the key, and grep -rni security src/ finds only unrelated prose. Applying this change locally leaves the committed snapshot byte-identical and the suite green (62/62), so the fixture cannot presently catch a regression in optional-security handling (for example a change that unconditionally wires an X-Api-Key header even where the spec allows anonymous access). It only becomes a real guard once security parsing is wired.

security:
- {}
- apiKey: []

Root cause:

#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
pub(crate) struct Operation {
pub(crate) operation_id: Option<String>,
#[serde(default)]
pub(crate) tags: Vec<String>,
#[serde(default)]
pub(crate) parameters: Vec<Parameter>,
pub(crate) request_body: Option<RequestBody>,
pub(crate) responses: Option<BTreeMap<String, Response>>,
pub(crate) summary: Option<String>,
pub(crate) description: Option<String>,
/// OpenAPI `deprecated: true` on the operation. Emitted as `@deprecated`
/// in the JSDoc above the service method so call sites surface the IDE
/// deprecation marker.
#[serde(default)]
pub(crate) deprecated: bool,
}

  1. The fixture's documenting comment was not updated and now understates what the fixture pins. It says the fixture confirms that components.securitySchemes blocks are silently accepted, but the fixture now also carries an operation-level security list with an anonymous {} alternative. Every fixture in this list is documented inline, so someone later wiring auth-aware emission will read this entry, conclude only the components block is covered, and either miss the anonymous-auth case the snapshot now pins or drop the - {} line as stray noise.

'nullable-oneof.openapi.yaml',
// security-schemes confirms that components.securitySchemes blocks are
// silently accepted (no error, no warning) — generation proceeds as if
// the block were not present. If we ever wire auth-aware emission this
// snapshot surfaces the change.
'security-schemes.openapi.yaml',
// circular-allof exercises the E4 recursion-guard happy path: 5 layers

  1. Pre-existing, not introduced by this PR; noting it as a follow-up since it is the same surface. security is dropped with no diagnostic, which is inconsistent with how every other dropped semantic is handled: format emits E_UNSUPPORTED_SEMANTIC with subcode format-dropped, and header / cookie parameters warn. The result is that a user generating a client for an apiKey-protected spec gets no auth wiring and a completely clean diagnostics list, and finds out at runtime via 401s. Given this PR's subject, a security-dropped subcode looks like the higher-value change; worth a separate issue rather than anything blocking here.

openapi-ng/index.d.ts.in

Lines 45 to 47 in d1bbe1b

| 'duplicate-schema-name'
| 'format-dropped'
| 'schema-cap-exceeded'

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

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.

2 participants