Skip to content

fix(security): harden auth — no default JWT secret, loopback bind, pin HS256 (H1+L1) - #11

Merged
willamhou merged 1 commit into
masterfrom
sec/pr-a-auth-hardening
Jun 24, 2026
Merged

fix(security): harden auth — no default JWT secret, loopback bind, pin HS256 (H1+L1)#11
willamhou merged 1 commit into
masterfrom
sec/pr-a-auth-hardening

Conversation

@willamhou

Copy link
Copy Markdown
Owner

First PR of the security-hardening plan (docs/design/security-hardening.md, from a whole-repo Codex review). Addresses the two auth findings.

H1 — default JWT secret + bind-all

  • Remove the dev-secret default for --jwt-secret; it is now required (flag or RELAIS_JWT_SECRET).
  • Reject the well-known dev-secret and any secret < 32 chars at startup, unless RELAIS_ALLOW_WEAK_JWT_SECRET=1 (canonical 1/true only — a per-area dev opt-out, not a global RELAIS_DEV).
  • Default bind 127.0.0.1; add --host / RELAIS_HOST. Warn on any unspecified (all-interfaces) address and bracket IPv6 literals ([::]:port).

L1 — JWT algorithm pinning

  • Validation::default()Validation::new(Algorithm::HS256) so none/RS256 alg-confusion tokens are rejected even if library defaults change; exp stays validated with explicit leeway.

Tests

  • New unit tests for validate_jwt_secret (rejects dev-secret/short, accepts strong, honors the opt-out).
  • Updated CLI parser tests: serve without a secret now must fail to parse; positive cases assert the host default + required secret.
  • cargo test --workspace green; cargo clippy -p relais-server -p relais-cli clean.
  • E2E verified: no-secret → clap error; dev-secret → refusal; RELAIS_ALLOW_WEAK_JWT_SECRET=1 → permitted with warning; default binds loopback; 0.0.0.0 warns.

Codex-reviewed (no BLOCKER/HIGH); the flagged stale CLI test and LOW cleanups are included.

🤖 Generated with Claude Code

…n HS256 (H1+L1)

PR-A of the security-hardening plan (docs/design/security-hardening.md).

- H1: remove the `dev-secret` default for `--jwt-secret` (now required via flag or
  RELAIS_JWT_SECRET); reject the well-known `dev-secret` and secrets <32 chars at
  startup unless RELAIS_ALLOW_WEAK_JWT_SECRET=1 (canonical 1/true only). Default
  bind to 127.0.0.1; add --host/RELAIS_HOST; warn on any unspecified (all-interfaces)
  address and bracket IPv6 literals.
- L1: pin JWT verification to `Validation::new(Algorithm::HS256)` (reject none/RS256)
  with explicit leeway.
- Update CLI parser tests for the new required secret + host field.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@willamhou
willamhou merged commit 09db872 into master Jun 24, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

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: 260e14cbe3

ℹ️ 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".

let cli = Cli::parse_from(["relais", "serve"]);
fn cli_serve_requires_jwt_secret() {
// No insecure default: `serve` without a secret must fail to parse.
assert!(Cli::try_parse_from(["relais", "serve"]).is_err());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Isolate serve parser tests from RELAIS_JWT_SECRET

When RELAIS_JWT_SECRET is set in the developer or CI environment, clap satisfies the required jwt_secret from the env var, so this assertion returns Ok even though no CLI flag was provided. Since this change makes RELAIS_JWT_SECRET the documented way to configure serve, the test will be flaky in exactly the environments likely to have that variable; clear/scope the relevant env vars for these parser tests before asserting the no-secret failure.

Useful? React with 👍 / 👎.

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.

1 participant