fix(pg-pkg)!: require explicit --allowed-origins for CORS (#216)#219
fix(pg-pkg)!: require explicit --allowed-origins for CORS (#216)#219dobby-coder[bot] wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Rules Dobby 2 — consolidated review (cycle 1)
The core change is correct and well-verified: --allowed-origins is now required = true with no default, so the PKG server refuses to start unless an operator explicitly supplies an allowlist. This directly closes the open-CORS pitfall flagged in the repo's core-security.md (the PKG issues private key material, so a silent * default was a real hazard). Runtime-exercised in review: bare start errors "required arguments were not provided"; env var and flag both work; wildcard * opt-in works; entrypoint.sh passes the env var through with no fallback; dev docker-compose.yml explicitly opts into *. Tests 37/37 green, fmt clean. Nice.
One blocking item (semver / changelog correctness):
Removing the default_value = "*" makes this an operationally breaking change — any deployment or bare-binary invocation that previously relied on the implicit default will now fail to start until it sets --allowed-origins / PKG_ALLOWED_ORIGINS. This repo uses release-plz (see the chore: release PR #187), which derives the version bump from the conventional-commit type. A fix: prefix cuts only a patch release and hides the breaking startup requirement from the changelog, so operators upgrading would be blindsided. Please signal the break:
- Retitle to a breaking type, e.g.
fix!(pg-pkg): require explicit --allowed-origins for CORS (#216)(orfeat!), and - add a
BREAKING CHANGE: pg-pkg server now refuses to start without --allowed-origins / PKG_ALLOWED_ORIGINS (previously defaulted to "*").line to the PR body/commit so release-plz records it and cuts a major bump.
See the inline comment on opts.rs.
Non-blocking (pre-existing, OUT OF SCOPE — do NOT fix in this PR):
The CORS-mode log lines in pg-pkg/src/server.rs (~L95/L99) are emitted before env_logger::init_from_env, so the "allowing any origin" warning is silently dropped — which undercuts operator visibility of a wildcard config. This is pre-existing (server.rs is unchanged vs main) and should be handled in its own PR; do not modify server.rs here. Noted for a human/follow-up only.
Remove `default_value = "*"` from the `--allowed-origins` / `PKG_ALLOWED_ORIGINS` clap option and mark it `required = true`, so the PKG server refuses to start unless the operator explicitly supplies an allowed-origins list. This prevents CORS from being silently open to all origins in production. The wildcard `*` is still accepted, but only as an explicit opt-in. entrypoint.sh now passes PKG_ALLOWED_ORIGINS through with no fallback (matching the existing IRMA_TOKEN idiom). The dev docker-compose sets PKG_ALLOWED_ORIGINS=* explicitly so local dev keeps working. BREAKING CHANGE: `--allowed-origins` / `PKG_ALLOWED_ORIGINS` is now required. The previous `*` default is gone, so any pg-pkg deployment that relied on the implicit wildcard must now set an allowlist explicitly (use `*` to keep the old open-CORS behaviour deliberately). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
90b83cc to
0b97d46
Compare
Closes #216.
Problem
pg-pkg's--allowed-origins/PKG_ALLOWED_ORIGINSoption defaulted to"*", so a PKG deployment that forgot to configure an allowlist would silently serve CORS to all origins — undesirable for a service that issues key material (see the linked security advisory GHSA-6x5w-5pjh-9966).Change
pg-pkg/src/opts.rs: removeddefault_value = "*"and marked the optionrequired = true. The server now refuses to start unless an allowlist is explicitly supplied. The wildcard*is still honoured, but only as a deliberate opt-in (the existingallow_any_originpath inserver.rsstill logs a warning when it's used).entrypoint.sh: passesPKG_ALLOWED_ORIGINSthrough with no fallback, using the same${VAR:+...}idiom already used forIRMA_TOKEN.docker-compose.yml: setsPKG_ALLOWED_ORIGINS=*explicitly for the dev service, so local development keeps working now that the option is required.--allowed-origins/PKG_ALLOWED_ORIGINSis now required. The previous*default is gone, so any existingpg-pkgdeployment that relied on the implicit wildcard must now set an allowlist explicitly (pass*to keep the old open-CORS behaviour deliberately). This is why the commit/PR uses thefix(pg-pkg)!:breaking type with aBREAKING CHANGE:footer, so release-plz cuts a major version bump.Verification
cargo build -p pg-pkg— clean.cargo test -p pg-pkg— 37 passed.cargo fmt --all -- --check— clean.pg-pkg serverwith no origins →error: the following required arguments were not provided: --allowed-origins <ALLOWED_ORIGINS>(exits before startup).PKG_ALLOWED_ORIGINS=https://a,https://b pg-pkg server ...→ parses and proceeds past CORS setup.--helpshows the option as required and readsPKG_ALLOWED_ORIGINS.🤖 Generated with Claude Code