-
Notifications
You must be signed in to change notification settings - Fork 0
Document security audit updates and refine server auth flow #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 148424c6bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (liveReloadEnabled) { | ||
| app.get("/__dev/reload", (req, res) => { | ||
| app.get("/__dev/reload", enforceLocalOrAuth, (req, res) => { | ||
| res.setHeader("Content-Type", "text/event-stream"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow live-reload when API token is set
When CONSENSUS_API_TOKEN is set, /__dev/reload now goes through enforceLocalOrAuth, which requires a bearer token even on localhost. The browser live-reload client does new EventSource('/__dev/reload') without any way to send headers or a ?token= query (see public/src/main.tsx), so in dev with CONSENSUS_LIVE_RELOAD=1 the SSE stream will be rejected with 401 and reloads silently stop. Consider adding a token query parameter to the EventSource URL or exempting localhost for this dev-only route even when a token is configured.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /(\"(?:password|passwd|pwd|api[_-]?key|api[_-]?token|access[_-]?token|auth[_-]?token|secret|client[_-]?secret|session[_-]?token)\"\\s*:\\s*\")([^\"]+)(\")/gi; | ||
| const STRICT_PATTERNS: Array<[RegExp, string]> = [ | ||
| [/AKIA[0-9A-Z]{16}/g, "<redacted-aws-key>"], | ||
| [/ASIA[0-9A-Z]{16}/g, "<redacted-aws-key>"], | ||
| [/ghp_[A-Za-z0-9]{36,}/g, "<redacted-github-token>"], | ||
| [/gho_[A-Za-z0-9]{36,}/g, "<redacted-github-token>"], | ||
| [/github_pat_[A-Za-z0-9_]{20,}/g, "<redacted-github-token>"], | ||
| [/xox[baprs]-[A-Za-z0-9-]{10,}/g, "<redacted-slack-token>"], | ||
| [/sk-[A-Za-z0-9]{20,}/g, "<redacted-openai-key>"], | ||
| [/AIza[0-9A-Za-z\\-_]{30,}/g, "<redacted-gcp-key>"], | ||
| [/eyJ[A-Za-z0-9_=-]+\\.[A-Za-z0-9._=-]+\\.[A-Za-z0-9._=-]+/g, "<redacted-jwt>"], | ||
| [ | ||
| /-----BEGIN (?:RSA|EC|DSA|OPENSSH|PGP) PRIVATE KEY-----[\\s\\S]+?-----END (?:RSA|EC|DSA|OPENSSH|PGP) PRIVATE KEY-----/g, | ||
| "<redacted-private-key>", | ||
| ], | ||
| [/Bearer\\s+[A-Za-z0-9._=-]+/gi, "Bearer <redacted-token>"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Incorrect regex escape sequences in strict redaction patterns cause secrets to not be redacted
Several regex patterns in the strict redaction mode use double backslashes (\\) instead of single backslashes (\) for escape sequences. In JavaScript regex literals, \\s matches a literal backslash followed by 's', not whitespace.
Click to expand
Affected patterns
The following patterns will NOT match their intended targets:
- JSON_KEY_PATTERN (line 14):
\\s*won't match whitespace between key and colon - JWT pattern (line 24):
\\.won't match the dots separating JWT segments - Private key pattern (line 26):
[\\s\\S]won't match any character (multiline content) - Bearer pattern (line 29):
\\s+won't match whitespace after 'Bearer' - GCP key pattern (line 23):
\\-won't match hyphens in keys
Example
// Current (broken):
/eyJ[A-Za-z0-9_=-]+\\.[A-Za-z0-9._=-]+\\.[A-Za-z0-9._=-]+/g
// This matches literal backslash + dot, not a dot
// Should be:
/eyJ[A-Za-z0-9_=-]+\.[A-Za-z0-9._=-]+\.[A-Za-z0-9._=-]+/gImpact
When CONSENSUS_REDACT_STRICT=1 is set, sensitive data like JWTs, Bearer tokens, and private keys will NOT be redacted as expected, potentially exposing secrets in logs and summaries.
Recommendation: Replace double backslashes with single backslashes in all regex escape sequences. For example: \\s → \s, \\. → \., [\\s\\S] → [\s\S]
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Testing
Ensure you read the contribution guide before submitting a commit and follow the exact guidelines.