Add trusted management origin allowlist for reverse-proxy logins - #33
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideIntroduces a validated, hot-reloadable allowlist for trusted management origins to support reverse-proxy logins without weakening same-origin or CSRF protections, with startup precedence/locking, deployment wiring, tests, and bilingual documentation. Sequence diagram for trusted-origin management authenticationsequenceDiagram
participant Browser
participant AdminAPI
participant AdminAuth
participant Settings
Browser->>AdminAPI: POST /admin/session
AdminAPI->>AdminAuth: csrf_enabled()
AdminAPI->>AdminAuth: allowed_origins()
AdminAuth->>Settings: Read admin_allowed_origins
Settings-->>AdminAuth: Normalized origin set
AdminAPI->>AdminAuth: same_origin(request, allowed_origins)
alt Same origin or exact allowed Origin
AdminAPI->>AdminAuth: login(request, api_key)
AdminAuth-->>Browser: 200 login response
else Origin rejected
AdminAPI-->>Browser: 403 Origin check failure
end
Sequence diagram for allowlisted management writes with CSRF protectionsequenceDiagram
participant Browser
participant AdminAPI
participant AdminAuth
participant Session
Browser->>AdminAPI: Cookie-authenticated management write
AdminAPI->>AdminAuth: allowed_origins()
AdminAuth-->>AdminAPI: Normalized allowlist
AdminAPI->>AdminAuth: same_origin(request, allowed_origins)
AdminAPI->>Session: Compare x-csrf-token with session csrf_token
alt Origin accepted and CSRF token valid
AdminAPI-->>Browser: Management response
else Origin or CSRF validation fails
AdminAPI-->>Browser: 403 validation failure
end
Flow diagram for allowed-origin normalizationflowchart TD
INPUT[Comma or space separated origins] --> COUNT{At most 32 entries?}
COUNT -- No --> INVALID[Reject setting]
COUNT -- Yes --> SCHEME[Add https to bare domains]
SCHEME --> VALIDATE{Valid http or https origin?}
VALIDATE -- No --> INVALID
VALIDATE -- Yes --> NORMALIZE[Normalize host and default ports]
NORMALIZE --> DEDUPE[Remove duplicates]
DEDUPE --> STORED[Store hot normalized value]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. This changes the trust boundary for admin login and cookie-authenticated writes by allowing configured browser origins to bypass the server-observed same-origin check. If the allowlist or proxy assumptions are wrong, an attacker-controlled origin could trigger administrative actions and obtain a session immediately; reverting prevents future requests but does not undo actions already performed.
dfdaa0b to
0ecb81d
Compare
Changes
admin_allowed_origins(WebUI system settings, hot) plusCODEBUDDY2API_ADMIN_ORIGINS/--admin-allowed-origins: comma-separated origins or bare domains (https://chat.example.com,chat.example.com; bare domains mean HTTPS), validated and normalized, at most 32 entries. An explicit CLI or environment value locks the WebUI field under the existing precedence rules.Verification
--admin-allowed-origins chat.example.com, http://10.0.0.1:8787, whitelisted-Origin logins return 200 while a missing or unlisted Origin stays 403, and the settings API reports the normalized value as locked with CLI source. Without the flag, a WebUI PATCH ofproxy.example.comnormalizes tohttps://proxy.example.com, applies hot (login then returns 200), reports sourcemanagement, and rejects an invalid entry with 400.admin_csrfescape hatch is unchanged.Scope and rollback
No change to same-origin matching rules, Referer fallback, CSRF tokens, session handling or
/v1/*; the allowlist only relaxes the browser Origin comparison for explicitly configured addresses. Remove the setting (or clear it in the WebUI) and the extra origins stop being trusted immediately; no persisted-state migration is needed.Summary by Sourcery
Support trusted management origins for reverse-proxy deployments without weakening the existing Origin and CSRF protections.
New Features:
Bug Fixes:
Enhancements:
Deployment:
Documentation:
Tests: