Skip to content

Implement AuthenticationMiddleware (#117)#171

Open
leogdion wants to merge 1 commit into
claude-promptfrom
117-authentication-middleware
Open

Implement AuthenticationMiddleware (#117)#171
leogdion wants to merge 1 commit into
claude-promptfrom
117-authentication-middleware

Conversation

@leogdion

Copy link
Copy Markdown
Member

Closes #117

Summary

Adds Sources/AiSTKit/AuthenticationMiddleware.swift — a public OpenAPI ClientMiddleware that injects the Anthropic authentication headers into every outgoing request:

  • x-api-key with the stored API key
  • anthropic-version: 2023-06-01

The request is then forwarded to the next handler via next(request, body, baseURL). The middleware will be wired into the generated ClaudeKit client by the ClaudeKit wrapper in #116.

Implementation notes

  • Implemented against the actual swift-openapi-runtime 1.x ClientMiddleware signature (includes body: HTTPBody? and returns (HTTPResponse, HTTPBody?)) — the simplified snippet in Docs/skit-analyze-plan.md §I2 predates the current protocol shape.
  • Uses public import for OpenAPIRuntime, HTTPTypes, and Foundation.URL since the package builds with InternalImportsByDefault and these types appear in the public conformance.
  • Avoids force-unwrapping HTTPField.Name (SwiftLint force_unwrapping rule) via if let on the always-valid header names.

Acceptance criteria

  • AuthenticationMiddleware implements ClientMiddleware protocol
  • intercept() adds x-api-key header with API key
  • intercept() adds anthropic-version header
  • Middleware correctly forwards request to next handler
  • Code follows one-type-per-file organization

Verification

  • swift build — clean
  • swift test — 489 tests pass
  • ./Scripts/lint.sh — SwiftFormat, SwiftLint, header check, and test build pass (Periphery's "index store path does not exist" error reproduces on a clean checkout, pre-existing local environment issue; Periphery is skipped in CI)

🤖 Generated with Claude Code

Adds a public ClientMiddleware that injects the x-api-key and
anthropic-version headers into every outgoing ClaudeKit request.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b0223f9-ea90-4fea-9fff-427fcd756beb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 117-authentication-middleware

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review: Implement AuthenticationMiddleware (#117)

Overview

Adds AuthenticationMiddleware, an OpenAPI ClientMiddleware that injects x-api-key and anthropic-version headers into every outgoing request. Clean, focused implementation — one type per file, correct protocol conformance, good use of public import scoping.


Issues

Silent header omission is a latent security bug

if let name = HTTPField.Name("x-api-key") {
    request.headerFields[name] = apiKey
}

HTTPField.Name.init is failable but will never return nil for these hardcoded, valid ASCII strings. The if let guard silently swallows a failure — if it ever fires, the request goes out without authentication headers, which is worse than crashing. The correct pattern is either:

  • guard let + throw a descriptive error, or
  • a force unwrap with a comment explaining these literals are always valid:
// Safety: these literals satisfy HTTPField.Name's ASCII constraints.
let apiKeyName = HTTPField.Name("x-api-key")!
let versionName = HTTPField.Name("anthropic-version")!

anthropicVersion not configurable

The API version is a private static constant. Anthropic does introduce breaking API version bumps. Consider making it injectable with a default value so callers can opt in to newer versions without modifying this type:

public init(apiKey: String, anthropicVersion: String = "2023-06-01") { ... }

Test Coverage

No tests are included. Suggested cases:

  • Verify both headers are present in the forwarded request.
  • Confirm the original body and base URL pass through unchanged.
  • Confirm the response tuple is returned unmodified.

A lightweight mock for the next closure that captures the mutated request makes these trivial to write.


Minor

  • File header says SyntaxKit in the module comment but this file lives in AiSTKit — cosmetic mismatch.

Overall this is a solid, minimal implementation. The silent-drop concern is the main thing worth addressing before merge.

🤖 Generated with Claude Code

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review — AuthenticationMiddleware (#117)

Overview

This PR adds a single-file OpenAPI ClientMiddleware that injects x-api-key and anthropic-version headers into every outgoing Anthropic API request. The implementation is clean and narrowly scoped.


What's Good

  • Correct public import usage under InternalImportsByDefault — avoiding these would cause build failures on the protocol conformance.
  • @preconcurrency guard for Linux Foundation.URL is the right platform-specific workaround.
  • Struct is simple, all-let state — implicit Sendable conformance is safe and correct.
  • The file header, license block, and doc-comment style all match the project's conventions.

Issues

1. No unit tests (required by CLAUDE.md)

CLAUDE.md step 3 in Adding New Syntax Elements requires unit tests in Tests/SyntaxKitTests/Unit/. No test file is included here, and the PR description only confirms existing tests still pass. At minimum, a test should verify:

  • That x-api-key is set to the provided key.
  • That anthropic-version is set to the expected constant.
  • That next is called with the modified request and the original body/baseURL.

A simple mock next closure is enough — no network required.

2. Silent auth bypass on header-name construction failure

if let name = HTTPField.Name("x-api-key") {
    request.headerFields[name] = apiKey
}

HTTPField.Name.init(_:) returns nil for invalid field names. For known-valid compile-time constants like "x-api-key" and "anthropic-version" this will never be nil, but if it somehow were, the request would be forwarded without authentication headers — a silent security regression rather than a loud failure.

Options:

  • Use the typed HTTPField.Name constants if HTTPTypes exposes them (e.g. HTTPField.Name.authorization-style), and fall back to a preconditionFailure / assertionFailure with a clear message for the custom names.
  • Or add a comment explicitly acknowledging the invariant so a future reader doesn't "fix" it by removing the guard.

3. Anthropic version is not configurable

anthropicVersion is a private static let with no way for callers to override it. When Anthropic releases a new API version, every consumer must update and redeploy the library. Consider accepting it as an optional init parameter with a default:

public init(apiKey: String, anthropicVersion: String = "2023-06-01") {
    self.apiKey = apiKey
    self.anthropicVersion = anthropicVersion
}

This keeps the current behaviour as the default while allowing integrators to pin to a newer version without waiting on a library release.

4. Minor: file header says "SyntaxKit"

//  AuthenticationMiddleware.swift
//  SyntaxKit        ← should be AiSTKit

The // <ModuleName> line in the header should match the target (AiSTKit), not the top-level package.


Summary

Severity Issue
Required Add unit tests per CLAUDE.md convention
Medium Silent auth bypass if HTTPField.Name init returns nil — at minimum document the invariant
Low Make anthropicVersion configurable via init
Nit Fix module name in file header

The core logic is correct and the implementation matches the protocol shape accurately. Addressing the missing tests and the silent-failure path are the blockers before merge.

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.95%. Comparing base (07f505b) to head (6e9df42).

Additional details and impacted files
@@                Coverage Diff                @@
##           claude-prompt     #171      +/-   ##
=================================================
- Coverage          80.16%   79.95%   -0.22%     
=================================================
  Files                153      153              
  Lines               5209     5209              
=================================================
- Hits                4176     4165      -11     
- Misses              1033     1044      +11     
Flag Coverage Δ
spm 79.95% <ø> (ø)
swift-6.1 ?
swift-6.2 ?
swift-6.3 79.94% <ø> (-0.14%) ⬇️
ubuntu 79.94% <ø> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review — PR 171: Implement AuthenticationMiddleware

Straightforward, correct implementation. One concern worth addressing.

Potential silent auth failure

The if let guards around HTTPField.Name initialisation will silently drop the authentication headers if the name string is ever considered invalid:

if let name = HTTPField.Name("x-api-key") {
    request.headerFields[name] = apiKey
}
// if name is nil, the request goes out WITHOUT the x-api-key header

The PR description explains this was chosen to satisfy SwiftLint's force_unwrapping rule, which is correct. However, a silent no-op on auth failure is a hard-to-debug failure mode — the request will succeed (from the network's perspective) but get a 401 from the API, with no indication at the call site that the header was never set.

Recommendation: Replace the if let with a guard ... else { preconditionFailure(...) } for these compile-time-constant strings. A preconditionFailure is stripped in release builds but will surface the bug immediately in any debug/test run. Alternatively, declare the HTTPField.Name values as private static let constants on the type so the failure is caught at type-load time. This requires a // swiftlint:disable:next force_unwrapping annotation plus a brief comment explaining why the force-unwrap is safe — a future typo in the header name will then fail fast at startup rather than silently sending unauthenticated requests.


Minor observations

  • Hardcoded anthropic-version: 2023-06-01: fine as a private static let constant; consider whether it belongs in a shared constants file alongside the model identifier in AnalyzerConfiguration.
  • public import usage for HTTPTypes, OpenAPIRuntime, and Foundation.URL is correct under Swift 6's InternalImportsByDefault since these types appear in the public API surface. ✓
  • @preconcurrency on Linux only matches the upstream behaviour of Foundation.URL's Sendable conformance availability. ✓
  • One-type-per-file, explicit access control — all consistent with project conventions. ✓

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.

1 participant