Skip to content

[Security] Reject cross-site and DNS-rebound requests to any keyless controller#242

Open
fettpl wants to merge 5 commits into
sybil-solutions:mainfrom
fettpl:p1/issue-222-controller-csrf
Open

[Security] Reject cross-site and DNS-rebound requests to any keyless controller#242
fettpl wants to merge 5 commits into
sybil-solutions:mainfrom
fettpl:p1/issue-222-controller-csrf

Conversation

@fettpl

@fettpl fettpl commented Jul 17, 2026

Copy link
Copy Markdown

Summary

Adds an explicit request-authority and browser-origin boundary to every controller posture without an API key. Hostile cross-site requests and DNS-rebound authorities are rejected before CORS, controller logging, observability, rate limiting, authentication, or route execution.

Closes #222

Dependency

This branch retains the two reviewed PR #244 prerequisite commits (ee9c5945 and 3d0270ff). The #244 package delta remains exactly the removal of the redundant direct playwright declaration from frontend/package.json and its root lockfile entry; @playwright/test continues to provide the transitive runtime.

Root cause

Without LOCAL_STUDIO_API_KEY, protected routes admitted every request. CORS controlled browser response visibility but did not prevent request execution, and the controller did not validate the request Host authority against the configured bind topology.

Solution design

  • Adds Effect Schema-validated LOCAL_STUDIO_ALLOWED_HOSTS configuration containing exact hostname or IP authorities.
  • Canonicalizes hostname, IPv4, and IPv6 forms while rejecting schemes, credentials, paths, wildcards, malformed authorities, and unexpected ports.
  • Derives safe loopback defaults for localhost, 127.0.0.1, ::1, and host.docker.internal.
  • Requires an explicit nonempty allowlist before a keyless 0.0.0.0 or :: bind can start.
  • Accepts Origin-less native clients only through an allowed authority.
  • Accepts browser requests only when their normalized HTTP(S) origin is configured in LOCAL_STUDIO_CORS_ORIGINS.
  • Does not trust Forwarded or X-Forwarded-Host; forwarded-host deployments require a separate explicit trust policy.
  • Leaves API-key-protected behavior unchanged.

Current-main refresh

The reviewed issue head cca1a89b259facefcf64fa48f4227f6a1439cfed was rebased through the reviewed PR #244 dependency at exact head 3cdd3a6402308c1d7a25816205cdedd7e73688f6 onto current main at 08bcba27f137f9c3d0206ea811d0aa7548a2dfa6, producing exact head fa8f537ad3ac7d7731475b84f3f8b55027f50031.

The two conflict resolutions preserve main's Effect runtime architecture:

  1. controllerRuntimeMiddleware(runtime) remains the first application middleware.
  2. createKeylessRequestGuardMiddleware(context) is immediately second.
  3. The guard remains ahead of CORS, request logging, controller-request observability, mutating and read rate limits, authentication, and all routes.
  4. The guard and the existing auth/rate-limit middleware use effectMiddleware and interruptible controller runtime execution.

The integration suite moved from controller/src/http/security-middleware.test.ts to controller/tests/integration/security-boundary.test.ts, with controller/tests/tsconfig.json providing strict test typing while keeping async test harness code outside the production standards scan.

Patch identity for the previously reviewed issue commits is unchanged:

  • 9882d98 -> 72abef6f4daccddbd8fb98260c7fce4cd6ef03ff
  • cca1a89 -> ae952844d5c6c448753c9e497be2f03aa44280ac

Validation

cd controller && bun test tests/integration/security-boundary.test.ts
13 passed, 0 failed, 94 assertions

cd controller && bunx tsc --noEmit -p tests/tsconfig.json
passed

cd controller && bun test --coverage tests/integration/security-boundary.test.ts
controller/src/config/env.ts: 95.00% functions, 94.71% lines
controller/src/http/security-middleware.ts: 86.67% functions, 83.42% lines

npm run check
passed (contracts, structure, frontend quality/build, controller quality/standards)

git diff --check origin/main...HEAD
passed

The normal pre-push hook passed conventional-commit validation and the complete frontend quality/production-build gate while publishing fa8f537ad3ac7d7731475b84f3f8b55027f50031.

Because the stacked dependency touches frontend package metadata, the production desktop distribution was also rebuilt and installed under the repository reinstall lock. The canonical /Applications/Local Studio.app verifies with a strict recursive hardened-runtime signature, retains bundle ID org.local.studio.desktop, and returned {"ok":true} from /api/desktop-health on repeated host-level probes.

UI changes

No visible UI changes. The frontend delta is limited to the reviewed one-line dependency cleanup from PR #244.

Risks / rollout notes

Keyless wildcard and LAN deployments must enumerate the exact authorities clients use. API-key deployments retain their existing authentication boundary. Reverse proxies are not trusted implicitly: deployments requiring forwarded-host semantics need a separate explicit trust policy rather than accepting client-supplied forwarding headers.

Workflow state

The refreshed fork-PR workflows require maintainer approval:

@fettpl
fettpl force-pushed the p1/issue-222-controller-csrf branch from 61086a4 to a4f0520 Compare July 17, 2026 19:55
@fettpl fettpl changed the title [Security] Reject cross-site and DNS-rebound requests to any keyless controller [Security] Guard keyless controller request authority Jul 17, 2026
@fettpl

fettpl commented Jul 17, 2026

Copy link
Copy Markdown
Author

@0xSero PR #242 is refreshed through the reviewed PR #244 dependency onto current main (08bcba27f137f9c3d0206ea811d0aa7548a2dfa6) at exact head fa8f537ad3ac7d7731475b84f3f8b55027f50031.

An independent P0/P1 review found no blockers. The 13-case security boundary suite, strict test typecheck, full repository gate, normal pre-push quality/build hook, and canonical desktop acceptance all passed. GitHub reports the PR mergeable, ready, and blocked only by review/workflow approval.

The exact-head workflows are action_required with no executable jobs until maintainer approval: CI run 29730768261 and Security run 29730768281. Please approve them when convenient.

@fettpl fettpl changed the title [Security] Guard keyless controller request authority [Security] Reject cross-site and DNS-rebound requests to any keyless controller Jul 18, 2026
@fettpl
fettpl force-pushed the p1/issue-222-controller-csrf branch from a4f0520 to cca1a89 Compare July 18, 2026 11:03
@fettpl
fettpl marked this pull request as ready for review July 18, 2026 11:06
@fettpl
fettpl requested a review from 0xSero as a code owner July 18, 2026 11:06
@fettpl
fettpl force-pushed the p1/issue-222-controller-csrf branch from 5c38056 to fa8f537 Compare July 20, 2026 09:15
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.

[Security] Reject cross-site and DNS-rebound requests to any keyless controller

1 participant