Skip to content

Require auth for proxy-forwarded Docker traffic - #4290

Merged
SpicyMarinara merged 1 commit into
stagingfrom
security/docker-proxy-auth-default
Jul 30, 2026
Merged

SpicyMarinara merged 1 commit into
stagingfrom
security/docker-proxy-auth-default

Conversation

@SpicyMarinara

@SpicyMarinara SpicyMarinara commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Linked issue

Closes #4284

Why this change

Docker's direct-container convenience bypass should not silently extend to proxy-forwarded traffic. A forwarding header marks a request as crossing a trust boundary, so the safer behavior is to enforce configured authentication and authorization gates unless the operator explicitly selects the legacy compatibility mode.

What changed

Validation

  • pnpm check passes locally
  • Container (Docker / Podman) built and ran without issue
  • Ran the app, clicked through the changes manually
  • Checked edge cases (light + dark mode, mobile viewport, empty states, error paths)
  • Above manual verification completed (describe below)
  • Read and followed CONTRIBUTING.md

Automated proof completed before publication:

  • pnpm regression:request-security
  • pnpm check
  • pnpm version:check
  • git diff --check staging...HEAD

Manual verification notes

  • Manually verify a proxied Docker deployment with password protection.
  • Manually verify IP allowlisting and privileged-route protection through the reverse proxy.
  • Manually verify direct container-local access and the explicit REQUIRE_AUTH_FOR_DOCKER_PROXY=false compatibility override.
  • No Docker container was built or exercised manually in this agent run.

Docs and release impact

  • No docs changes needed
  • Updated docs (README / CONTRIBUTING / android/README / CHANGELOG) as needed
  • Translated docs on the docs-i18n branch updated to match, or a [docs-i18n] follow-up issue opened (see CONTRIBUTING.md § Translated documentation)
  • Version/release files updated (only if this PR includes a version bump)

English documentation and CHANGELOG.md are updated. Translation follow-up: #4289.

UI evidence (if applicable)

Not applicable; the behavior is server middleware and deployment configuration.

Summary by CodeRabbit

  • Security

    • Docker traffic forwarded through a proxy now requires normal authentication and IP allowlist checks by default.
    • Direct Tailscale and same-host Docker bridge traffic remains trusted by default.
    • Legacy passwordless proxy behavior remains available through an explicit opt-out setting.
  • Documentation

    • Updated security, configuration, remote-access, and troubleshooting guidance to explain the new defaults and trust requirements.
  • Tests

    • Added regression coverage for proxy authentication, allowlists, Basic Auth, and privileged access.

@SpicyMarinara

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 98bac9b3-8b0c-44dd-8aa3-7ed1995f9cc9

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
📝 Walkthrough

Walkthrough

Proxy-forwarded Docker traffic now requires normal authorization by default. Direct Docker traffic retains its bypass behavior, while an explicit environment flag restores legacy compatibility. Regression coverage and documentation are updated accordingly.

Changes

Docker proxy authentication

Layer / File(s) Summary
Enforce forwarded Docker authentication
packages/server/src/config/runtime-config.ts, packages/server/src/middleware/*
Docker proxy authentication is required unless explicitly disabled; related middleware documentation and warning text reflect the new behavior.
Validate authorization paths
scripts/regressions/docker-proxy-auth.regression.ts, package.json
Regression coverage verifies forwarded-request classification, allowlist checks, Basic Auth, privileged access, and environment restoration.
Align defaults and documentation
.env.example, docs/*, README.md, CHANGELOG.md
Configuration defaults and operator guidance describe forwarded Docker authentication, direct Docker bypasses, and the legacy opt-out.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: thelonelydevil9, lukathehero

Sequence Diagram(s)

sequenceDiagram
  participant DockerClient
  participant MarinaraAuthPolicy
  participant IPAllowlist
  participant BasicAuth
  participant PrivilegedGate
  DockerClient->>MarinaraAuthPolicy: Send forwarded Docker request
  MarinaraAuthPolicy->>IPAllowlist: Require normal allowlist checks
  IPAllowlist->>BasicAuth: Continue authorization
  BasicAuth->>PrivilegedGate: Provide authenticated context
  PrivilegedGate-->>DockerClient: Allow or reject privileged request
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: requiring auth for proxy-forwarded Docker traffic.
Description check ✅ Passed The description covers the linked issue, motivation, key changes, validation, and docs impact with the required sections present.
Linked Issues check ✅ Passed The changes satisfy #4284 by defaulting forwarded Docker traffic to normal auth checks, keeping direct-container bypass, and adding regressions/docs.
Out of Scope Changes check ✅ Passed The added docs, config, test script, and regression coverage all align with the stated security-hardening scope.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/docker-proxy-auth-default

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.

@SpicyMarinara
SpicyMarinara merged commit d1c3e01 into staging Jul 30, 2026
13 checks passed
@SpicyMarinara
SpicyMarinara deleted the security/docker-proxy-auth-default branch July 30, 2026 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant