security(packages/core): harden auth middleware against insecure transport and injection

[zrosenbauer]

Summary

• Enforce HTTPS on OAuth endpoint URLs (authUrl, tokenUrl, deviceAuthUrl) per RFC 8252 §8.3, allowing HTTP only for loopback addresses (127.0.0.1, [::1], localhost) used during local redirect flows
• Escape cmd.exe metacharacters in URLs passed to cmd /c start on Windows to prevent command injection via query strings
• Remove redundant existsSync check in loadFromPath to eliminate a TOCTOU race condition

Changes

• oauth-server.ts: Add exported isSecureAuthUrl() with private isLoopbackHost() helper; add private escapeCmdMeta() helper and apply it to the win32 branch in openBrowser()
• strategies/oauth.ts: Validate authUrl and tokenUrl with isSecureAuthUrl() at the top of resolveFromOAuth(); return null for insecure URLs
• strategies/device-code.ts: Validate deviceAuthUrl and tokenUrl with isSecureAuthUrl() at the top of resolveFromDeviceCode(); return null for insecure URLs
• create-store.ts: Remove existsSync guard in loadFromPath(), relying on attempt() to catch read errors
• Tests: Add 11 new tests covering isSecureAuthUrl (7), HTTP rejection in resolveFromOAuth (2) and resolveFromDeviceCode (2), and Windows metacharacter escaping in openBrowser (1)

Testing

1. pnpm typecheck — no type errors
2. pnpm test — all 456 tests pass (34 test files)
3. pnpm lint — 0 errors (no new warnings introduced)

Summary by CodeRabbit

• Bug Fixes

  • Enforced secure OAuth endpoint handling: only HTTPS allowed remotely; HTTP allowed for loopback/local redirects. Non-secure endpoints now return null.
  • Prevented Windows command-injection by escaping special characters in URLs passed to system open commands.

• Tests

  • Added unit tests covering URL security rules and Windows URL-escaping behavior.

• Documentation

  • Added changelog entry describing the security hardening.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds OAuth endpoint security checks (require HTTPS except for loopback HTTP), escapes cmd.exe metacharacters in URLs on Windows, and removes a pre-check to avoid a TOCTOU race when loading files.

Changes

Cohort / File(s)	Summary
OAuth server & tests packages/core/src/middleware/auth/oauth-server.ts, packages/core/src/middleware/auth/oauth-server.test.ts	Adds exported isSecureAuthUrl(url) and helpers (isLoopbackHost, escapeCmdMeta); updates openBrowser to escape Windows cmd metacharacters; tests for HTTPS, loopback/non-loopback HTTP, IPv4/IPv6 localhost, invalid URLs, and Windows cmd escaping.
OAuth strategy & tests packages/core/src/middleware/auth/strategies/oauth.ts, packages/core/src/middleware/auth/strategies/oauth.test.ts	Adds early-return guards in resolveFromOAuth() using isSecureAuthUrl() to return null when authUrl or tokenUrl is not secure; tests cover HTTP rejection.
Device-code strategy & tests packages/core/src/middleware/auth/strategies/device-code.ts, packages/core/src/middleware/auth/strategies/device-code.test.ts	Adds early-return guards in resolveFromDeviceCode() using isSecureAuthUrl() to return null when deviceAuthUrl or tokenUrl is not secure; tests added for these cases.
Store loading packages/core/src/lib/store/create-store.ts	Removes explicit existsSync() check in loadFromPath() to avoid a TOCTOU race; relies on readFileSync() error handling and returns null on error.
Changelog .changeset/auth-security-hardening.md	New changelog entry documenting OAuth URL security hardening, Windows cmd metacharacter escaping, and the TOCTOU fix in file loading.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant OAuthStrategy
  participant OAuthServer
  participant OS

  Client->>OAuthStrategy: start OAuth/device-code flow (authUrl, tokenUrl)
  OAuthStrategy->>OAuthServer: call isSecureAuthUrl(authUrl)
  OAuthServer-->>OAuthStrategy: true/false (HTTPS or loopback HTTP)
  OAuthStrategy->>OAuthServer: call isSecureAuthUrl(tokenUrl)
  OAuthServer-->>OAuthStrategy: true/false
  alt any URL not secure
    OAuthStrategy-->>Client: return null (abort flow)
  else all URLs secure
    OAuthStrategy->>OAuthServer: generate PKCE / device code
    OAuthStrategy->>OS: openBrowser(url)
    OS->>OAuthServer: on Windows, URL was escaped via escapeCmdMeta
    OAuthServer-->>OS: spawn browser command
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I hopped through URLs, checked loopback and host,
Escaped every ampersand lest commands be lost,
No race on the path, files read with care,
Secure auth, safe hops — I tidy the lair! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main security improvements: hardening auth middleware against insecure transport and injection attacks, which directly aligns with all the major changes in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch security/auth-middleware-hardening

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/middleware/auth/oauth-server.ts`:
- Line 210: The escapeCmdMeta function used in the win32 branch (called from
.with('win32', () => ({ args: ['/c', 'start', '', escapeCmdMeta(url)], command:
'cmd' }))) is incomplete: extend it to also neutralize '%' and both '(' and ')'
characters (either by prefixing each with '^' like the other metacharacters or
by returning a properly quoted URL that prevents cmd.exe variable
expansion/grouping) while preserving existing escaping for &|<>^; update
escapeCmdMeta to consistently escape '%' and parentheses or wrap the final URL
in safe quotes so start receives a literal URL.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c1ef2360-1b7c-4a5a-95be-93e653058ab7

📥 Commits

Reviewing files that changed from the base of the PR and between c60e0b9 and 5792ba3.

📒 Files selected for processing (1)

• packages/core/src/middleware/auth/oauth-server.ts