Skip to content

fix(security): tighten secret-file perms, optional key pepper, validate allowedIps, proxy-aware queue-UI auth#345

Closed
rmyndharis wants to merge 1 commit into
mainfrom
fix/secrets-auth-hardening
Closed

fix(security): tighten secret-file perms, optional key pepper, validate allowedIps, proxy-aware queue-UI auth#345
rmyndharis wants to merge 1 commit into
mainfrom
fix/secrets-auth-hardening

Conversation

@rmyndharis

Copy link
Copy Markdown
Owner

Five small secrets/auth hardening fixes (grouped — all touch the credential/access-control surface). Off by default where opt-in; no breaking changes.

1. Generated secret files are owner-only (0600)

data/.env.generated (S3/DB/Redis credentials) and data/.api-key (the raw admin key) were created world-readable (0644). They are now 0600. writeFileSync's mode only applies on create, so writeSecretFile chmods before and after the write — an overwrite of a pre-existing loose file never leaves the new secret briefly world-readable — and startup tightens any pre-existing file.

2. Optional API-key pepper

Set API_KEY_PEPPER to hash API keys with HMAC-SHA256 instead of plain SHA-256, so a leak of the key table alone can't precompute candidates. Off by default — when unset the hashing is bit-identical to before, so existing stored keys keep validating. Enabling it is a deploy-time choice that invalidates keys hashed before it was set (re-issue keys).

3. allowedIps is validated (IPv4 / IPv4 CIDR)

A key's allowedIps was any string array; a typo'd entry was stored silently and never matched. Entries are now validated per element as an IPv4 address or IPv4 CIDR.

IPv6 is deliberately rejected: the runtime matcher (isIpAllowed) is IPv4-only, so an IPv6 entry can't be enforced — a /128 host-lock would never match its own client, and an IPv6 /≤32 CIDR would match every IPv6 client (an over-broad grant). Validating it as "good" would sanction an unenforceable, potentially dangerous restriction, so the validator fails closed and allowedIps is documented as IPv4-only.

4. Queue dashboard auth uses the same client-IP model as the API

Bull Board (/admin/queues) is mounted as raw Express middleware, outside the ApiKeyGuard. Its auth middleware read the socket address directly, so an allowedIps-restricted ADMIN key was enforced differently there than on the API. It now resolves the client IP through the shared resolveClientIp(req, trustedProxies) (honoring TRUSTED_PROXIES), so the allowlist is enforced consistently behind a reverse proxy — and a spoofed X-Forwarded-For is ignored when no trusted proxy is configured.

5. Production secret-guard checks the S3 vars the app actually uses

The fail-fast "no default secrets in production" guard inspected S3_ACCESS_KEY/S3_SECRET_KEY, but the storage layer reads S3_ACCESS_KEY_ID/S3_SECRET_ACCESS_KEY first. The guard now checks the canonical names (with the legacy names as fallback), matching the storage layer — closing a gap where a placeholder in the canonical var went unchecked.

Tests

New/updated unit tests for: secret-file perms (new file owner-only + pre-existing tightened); hashApiKey pepper vs. SHA-256 fallback and the service wiring (asserts the keyHash queried with HMAC when the pepper is set, SHA-256 when unset); the allowedIps validator (IPv4 accepted, malformed/IPv6 rejected) including the DTO @Validate({ each: true }) array path and a non-string element; Bull Board IP (XFF honored only behind a trusted proxy, ignored otherwise). Full backend 761/761; dashboard build + lint clean.

An adversarial review of the first cut caught a real bug: the validator initially accepted IPv6/IPv6-CIDR that the IPv4-only matcher can't enforce (a /≤32 IPv6 CIDR would have matched the entire IPv6 internet). Fixed by failing closed (IPv4-only) before this was opened.

…te allowedIps, proxy-aware queue-UI auth

- data/.env.generated and data/.api-key are written 0600 (chmod before+after write so an overwrite never exposes a world-readable window) and any pre-existing looser file is tightened on startup.

- API_KEY_PEPPER (opt-in) hashes API keys with HMAC-SHA256; unset keeps plain SHA-256 so existing stored hashes keep validating.

- allowedIps entries are validated as an IPv4 address or IPv4 CIDR. The runtime matcher is IPv4-only, so IPv6 is rejected rather than silently sanctioned as an unenforceable (and for a /<=32 CIDR, dangerously over-broad) entry.

- The Bull Board queue UI resolves the client IP via the shared trusted-proxy resolver, so an allowedIps-restricted ADMIN key is enforced consistently behind a reverse proxy, matching the API guard.

- The production startup secret-guard inspects S3_ACCESS_KEY_ID/S3_SECRET_ACCESS_KEY (canonical, legacy fallback) to match the storage layer.
rmyndharis added a commit that referenced this pull request Jun 19, 2026
* fix(webhook): deliver session lifecycle events and key webhook hardening (#335)

* fix(security): pin outbound webhook and media fetches to validated IP (#338)

* fix(plugins): persist plugin enable/config and restore (#339)

* fix(message): persist bulk-sent messages, sanitize SSRF (#340)

* fix(engine): return the real id for forwarded messages (#341)

* fix(security): harden outbound requests, IPv6 SSRF (#344)

* fix(security): secret-file perms, key pepper, allowedIps (#345)

* fix(storage): bound tar imports, contain storage keys (#346)

* fix(session): reconcile late acks, serialize reactions (#348)

* fix(contract): webhook timeout, bounded shutdown, 501 (#350)

* feat(session): force-kill a stuck session (#352)

* merge #343

* merge #351

* chore(release): v0.4.3 — CHANGELOG, version bump (package.json/dashboard/swagger), README + docs
@rmyndharis

Copy link
Copy Markdown
Owner Author

Shipped in v0.4.3 (integrated via the release PR #354 and tagged v0.4.3). Thanks! 🎉

@rmyndharis rmyndharis closed this Jun 19, 2026
@rmyndharis rmyndharis deleted the fix/secrets-auth-hardening branch June 19, 2026 18:33
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