Repository navigation
fix(web): only DANGEROUSLY_OMIT_AUTH=true/1 disables /api auth - #2390
Merged
Merged
Conversation
!!process.env.DANGEROUSLY_OMIT_AUTH read every non-empty string as on, so =false silently disabled the bearer gate. Reuse the DANGEROUSLY_BIND_ALL_INTERFACES parser (exported as isEnvFlagEnabled) so both safety flags fail closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>
There was a problem hiding this comment.
🟢 Approval recommended
The implementation matches the issue requirements and includes comprehensive regression coverage.
Pull request overview
Fixes fail-closed parsing for the web backend’s auth-disable flag.
Changes:
- Shares explicit
true/1environment-flag parsing. - Adds regression coverage for accepted and rejected values.
- Updates environment and migration documentation.
File summaries
| File | Description |
|---|---|
docs/v1-to-v2-migration.md |
Documents stricter v2 parsing. |
docs/environment-variables.md |
Clarifies accepted flag values. |
clients/web/src/test/integration/server/web-server-config.test.ts |
Tests opt-in and fail-closed cases. |
clients/web/server/web-server-config.ts |
Applies safe parsing to auth omission. |
clients/web/server/resolve-bind-host.ts |
Exports the shared flag parser. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2331
buildWebServerConfigFromEnvread the flag as!!process.env.DANGEROUSLY_OMIT_AUTH, so every non-empty string — includingfalseand0— turned the/api/*bearer gate off and printedAuth: disabled. A deployment that set=falseto keep auth on got the opposite.Changes
resolve-bind-host.ts: theDANGEROUSLY_BIND_ALL_INTERFACESparser (already fail-closed: only trimmed, case-insensitivetrue/1enable it) is exported asisEnvFlagEnabled, so bothDANGEROUSLY_*safety flags parse one way.web-server-config.ts:DANGEROUSLY_OMIT_AUTHnow goes through it.it.eachover the opt-in spellings (true,TRUE,True,1,1) and the ones that must keep auth on (false,FALSE,0, empty, whitespace,no,yes,on,2), asserting bothdangerouslyOmitAuthand that the token survives.docs/environment-variables.mdno longer warns thatfalsedisables auth; the v1→v2 migration table notes the stricter parsing (v1 also used!!, verified onv1/main).yesoronused to disable auth and now keeps it on. That is the fail-closed direction, and the docs have only ever shown=true.npm run local:gatepasses. No UI change, so no screenshots.🤖 Generated with Claude Code