🔒 security: harden SSRF, WS origins, auth fail-closed, store perms, session cookie, CSP, CI secret scans - #567
Merged
Merged
Conversation
scttbnsn
requested review from
ALARGECOMPANY and
biggest-littlest
as code owners
July 20, 2026 18:44
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds Gitleaks CI scanning, hardens session and anonymous authentication, validates WebSocket origin protocols, enforces restrictive container and store permissions, strengthens HTTP-trigger DNS handling, pins icon sources, introduces nonce-based CSP and JSON-LD rendering, and updates dependency security tests. Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
app/store/index.ts (2)
49-54: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
fs.existsSyncfollowed byfs.chmodSynccreates a time-of-check to time-of-use (TOCTOU) race condition. Handle theENOENTerror directly.♻️ Proposed fix
function enforceStorePermissions(storeDirectory: string, storePath: string): void { fs.chmodSync(storeDirectory, STORE_DIRECTORY_MODE); - if (fs.existsSync(storePath)) { - fs.chmodSync(storePath, STORE_FILE_MODE); - } + try { + fs.chmodSync(storePath, STORE_FILE_MODE); + } catch (error: any) { + if (error.code !== 'ENOENT') throw error; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/store/index.ts` around lines 49 - 54, Update enforceStorePermissions to call fs.chmodSync on storePath directly without the preceding fs.existsSync check, and catch only ENOENT errors to preserve the behavior when the file is absent while propagating other filesystem errors.
157-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the umask change
process.umask(0o077)is process-wide; if only Loki needs it, move it to the store path or set explicit modes on the other runtime file writers instead of changing the global default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/store/index.ts` around lines 157 - 162, Scope the restrictive umask configuration currently applied in the isMemoryMode block so it does not alter the process-wide default for unrelated writers. Move the permission handling into the Loki store path, or apply explicit file modes to Loki’s temporary and replacement files, while preserving owner-readable-only permissions for Loki persistence.app/triggers/providers/mqtt/Hass.ts (1)
163-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
normalizeSlugfromicons/providers.tsinstead of duplicating it.
normalizeIconSlugreimplements the same lowercase+strip-suffix logic already exported asnormalizeSlugfrom the module this file already imports (iconProviders). Two copies of the same normalization rule will drift if one changes.As per path instructions, "Notification trigger providers follow a shared provider pattern. Flag divergence from existing triggers."
♻️ Proposed fix
-import { providers as iconProviders } from '../../../api/icons/providers.js'; +import { normalizeSlug, providers as iconProviders } from '../../../api/icons/providers.js';-function normalizeIconSlug(slug: string, extension: string): string { - const normalizedSlug = slug.trim().toLowerCase(); - const suffix = `.${extension}`; - if (normalizedSlug.endsWith(suffix)) { - return normalizedSlug.slice(0, -suffix.length); - } - return normalizedSlug; -} -const cdn = cdnMap[provider as keyof typeof cdnMap]; - const slug = normalizeIconSlug(rawSlug, cdn.extension); + const slug = normalizeSlug(rawSlug, cdn.extension); return cdn.url(slug);Also applies to: 188-196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/triggers/providers/mqtt/Hass.ts` around lines 163 - 170, Remove the duplicate normalizeIconSlug implementation and reuse the exported normalizeSlug from iconProviders wherever icon slug normalization is needed, passing the appropriate slug and extension arguments. Update both referenced call sites to use the shared helper and preserve the existing normalized output.Source: Path instructions
apps/demo/tests/security/yaml-lockfile.test.js (1)
26-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDRY violation in security tests. Security tests duplicate
compareSemver, override assertions, and lockfile traversal logic. This prevents centralized test fixes (like nested path checks) and creates maintenance overhead.
apps/demo/tests/security/yaml-lockfile.test.js#L26-L37: extract override and lockfile validation into a shared utility.e2e/tests/security/brace-expansion-lockfile.test.js#L27-L31: consume the shared test utility instead of duplicating test logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/demo/tests/security/yaml-lockfile.test.js` around lines 26 - 37, Extract the duplicated semver, package override, and lockfile traversal checks from apps/demo/tests/security/yaml-lockfile.test.js lines 26-37 into a shared security-test utility, preserving validation of vulnerable nested lockfile entries. Update e2e/tests/security/brace-expansion-lockfile.test.js lines 27-31 to consume that utility instead of maintaining its own logic; both sites should use the centralized helper for their respective package and minimum-safe-version checks.app/api/auth.test.ts (1)
1091-1091: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDRY violation on cookie name.
Hardcoded cookie name. Import
SESSION_COOKIE_NAMEfrom./session-cookie.js. Apply this to the mock cookie header returns below as well.♻️ Proposed fix
- expect(sessionConfig.name).toBe('drydock.sid'); + expect(sessionConfig.name).toBe(SESSION_COOKIE_NAME);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/auth.test.ts` at line 1091, Replace the hardcoded 'drydock.sid' values in the auth tests, including the sessionConfig.name assertion and mock cookie header returns, with the imported SESSION_COOKIE_NAME constant from ./session-cookie.js.app/authentications/providers/anonymous/Anonymous.ts (1)
19-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDRY violation on guidance string.
Extract shared authentication guidance string into a constant.
♻️ Proposed fix
+ const guidance = 'Set DD_AUTH_BASIC_<name>_USER / DD_AUTH_BASIC_<name>_HASH to secure the dashboard, or set DD_ANONYMOUS_AUTH_CONFIRM=true to explicitly allow anonymous access.'; if (isUpgrade()) { throw new Error( - 'No authentication configured during an upgrade. Set DD_AUTH_BASIC_<name>_USER / DD_AUTH_BASIC_<name>_HASH to secure the dashboard, or set DD_ANONYMOUS_AUTH_CONFIRM=true to explicitly allow anonymous access.', + `No authentication configured during an upgrade. ${guidance}`, ); } throw new Error( - 'No authentication configured and this is a fresh install. Set DD_AUTH_BASIC_<name>_USER / DD_AUTH_BASIC_<name>_HASH to secure the dashboard, or set DD_ANONYMOUS_AUTH_CONFIRM=true to allow anonymous access.', + `No authentication configured and this is a fresh install. ${guidance}`, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/authentications/providers/anonymous/Anonymous.ts` around lines 19 - 27, In the anonymous-authentication error handling of Anonymous, extract the repeated authentication configuration guidance into a shared constant and reuse it in both upgrade and fresh-install error messages. Preserve each message’s distinct context and wording about explicitly allowing anonymous access.app/api/openapi.test.ts (1)
60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDRY violation. Use
SESSION_COOKIE_NAMEinstead of hardcoding the cookie name.♻️ Proposed fix
- name: 'drydock.sid', + name: SESSION_COOKIE_NAME,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/openapi.test.ts` around lines 60 - 64, Update the security scheme assertion in the OpenAPI test to use the existing SESSION_COOKIE_NAME constant for the cookie name instead of hardcoding 'drydock.sid', while preserving the other expected properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/store/index.ts`:
- Around line 213-221: Update the store initialization state to retain the exact
storeDirectory created by init(), alongside storePathResolved. In the
permission-check flow, remove the path.dirname derivation and pass the tracked
initialized directory to enforceStorePermissions while continuing to use
persistentStorePath for the file path.
In `@apps/demo/tests/security/yaml-lockfile.test.js`:
- Around line 35-37: Update the vulnerableEntries filtering in the lockfile
security test to match yaml package entries at any dependency depth, including
paths such as node_modules/foo/node_modules/yaml, while retaining the existing
version-type and compareSemver checks against MINIMUM_SAFE_YAML_VERSION.
In `@apps/web/src/proxy.ts`:
- Around line 1-6: Rename apps/web/src/proxy.ts to apps/web/src/middleware.ts,
export the handler as middleware, and use btoa(crypto.randomUUID()) instead of
Node.js Buffer. In apps/web/scripts/content-security-policy.test.mjs lines 7 and
44-57, update the source path, variable, test name, and assertions to
middlewareSource; update the middleware references in apps/web/next.config.mjs
line 28 and apps/web/scripts/next-config.test.mjs lines 15-16.
---
Nitpick comments:
In `@app/api/auth.test.ts`:
- Line 1091: Replace the hardcoded 'drydock.sid' values in the auth tests,
including the sessionConfig.name assertion and mock cookie header returns, with
the imported SESSION_COOKIE_NAME constant from ./session-cookie.js.
In `@app/api/openapi.test.ts`:
- Around line 60-64: Update the security scheme assertion in the OpenAPI test to
use the existing SESSION_COOKIE_NAME constant for the cookie name instead of
hardcoding 'drydock.sid', while preserving the other expected properties.
In `@app/authentications/providers/anonymous/Anonymous.ts`:
- Around line 19-27: In the anonymous-authentication error handling of
Anonymous, extract the repeated authentication configuration guidance into a
shared constant and reuse it in both upgrade and fresh-install error messages.
Preserve each message’s distinct context and wording about explicitly allowing
anonymous access.
In `@app/store/index.ts`:
- Around line 49-54: Update enforceStorePermissions to call fs.chmodSync on
storePath directly without the preceding fs.existsSync check, and catch only
ENOENT errors to preserve the behavior when the file is absent while propagating
other filesystem errors.
- Around line 157-162: Scope the restrictive umask configuration currently
applied in the isMemoryMode block so it does not alter the process-wide default
for unrelated writers. Move the permission handling into the Loki store path, or
apply explicit file modes to Loki’s temporary and replacement files, while
preserving owner-readable-only permissions for Loki persistence.
In `@app/triggers/providers/mqtt/Hass.ts`:
- Around line 163-170: Remove the duplicate normalizeIconSlug implementation and
reuse the exported normalizeSlug from iconProviders wherever icon slug
normalization is needed, passing the appropriate slug and extension arguments.
Update both referenced call sites to use the shared helper and preserve the
existing normalized output.
In `@apps/demo/tests/security/yaml-lockfile.test.js`:
- Around line 26-37: Extract the duplicated semver, package override, and
lockfile traversal checks from apps/demo/tests/security/yaml-lockfile.test.js
lines 26-37 into a shared security-test utility, preserving validation of
vulnerable nested lockfile entries. Update
e2e/tests/security/brace-expansion-lockfile.test.js lines 27-31 to consume that
utility instead of maintaining its own logic; both sites should use the
centralized helper for their respective package and minimum-safe-version checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8cd81a40-be60-4fd4-b29e-17d6ec59fddd
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (47)
.github/tests/ci-verify-workflow.test.ts.github/workflows/ci-verify.yml.gitleaks.toml.gitleaksignoreDocker.entrypoint.shDockerfileapp/api/auth.test.tsapp/api/auth.tsapp/api/container/log-stream.test.tsapp/api/csrf.test.tsapp/api/icons.test.tsapp/api/icons/fetch.test.tsapp/api/icons/providers.test.tsapp/api/icons/providers.tsapp/api/log-stream.test.tsapp/api/openapi.test.tsapp/api/openapi/index.tsapp/api/session-cookie.tsapp/api/ws-upgrade-utils.test.tsapp/api/ws-upgrade-utils.tsapp/authentications/providers/anonymous/Anonymous.test.tsapp/authentications/providers/anonymous/Anonymous.tsapp/configuration/dockerfile-defaults.test.tsapp/registry/index.test.tsapp/registry/index.tsapp/store/index.test.tsapp/store/index.tsapp/triggers/providers/http/Http.test.tsapp/triggers/providers/http/Http.tsapp/triggers/providers/mqtt/Hass.test.tsapp/triggers/providers/mqtt/Hass.tsapps/demo/tests/security/yaml-lockfile.test.jsapps/web/next.config.mjsapps/web/scripts/content-security-policy.test.mjsapps/web/scripts/next-config.test.mjsapps/web/src/app/compare/page.tsxapps/web/src/app/docs/[[...slug]]/page.tsxapps/web/src/app/layout.tsxapps/web/src/app/page.tsxapps/web/src/app/security/trivy-supply-chain-march-2026/page.tsxapps/web/src/components/comparison-page.tsxapps/web/src/components/json-ld.tsxapps/web/src/lib/content-security-policy.mjsapps/web/src/proxy.tsapps/web/vercel.jsone2e/tests/security/brace-expansion-lockfile.test.jsscripts/scan-secrets.sh
💤 Files with no reviewable changes (1)
- apps/web/vercel.json
…checks CodeRabbit review fixes on PR #567: - store: track storeDirectoryResolved alongside storePathResolved so the post-save permission repair chmods the configured DD_STORE_PATH root instead of re-deriving it via path.dirname(storeFile), which was wrong whenever DD_STORE_FILE names a subpath - store: replace the TOCTOU existsSync+chmod check in enforceStorePermissions with a direct chmodSync wrapped in try/catch that swallows ENOENT and rethrows everything else - security tests: lockfile filters in apps/demo/tests/security/yaml-lockfile.test.js and e2e/tests/security/yaml-lockfile.test.js matched only the top-level node_modules/yaml entry; now also match nested copies via path.endsWith('/node_modules/yaml') - Hass.ts: drop the local normalizeIconSlug duplicate of api/icons/providers.ts's normalizeSlug, trimming at the call site to preserve identical behavior
Contributor
Author
|
Swept the nitpicks too, in 9c59f38 where taken:
|
scttbnsn
added a commit
that referenced
this pull request
Jul 20, 2026
…delta (#569) ## What Docs/copy freshness pass for the v1.6.1 delta (the #567 security batch, the #566 display batch, and the #568 maturity-clock fix). 30 findings from an adversarially-verified audit workflow (8 surface auditors, every finding independently re-verified against the shipped code before it counted; 1 rejected in verification), applied as one coherent edit per doc region: - **Anonymous-auth fail-closed (A1)**: the authentications page (callout, behavior matrix, and the "Upgrading from v1.3.x" section), the security guide, the README banner + quickstart line, and DEPRECATIONS.md all still promised upgraders a warn-and-serve grace period. Rewritten everywhere: upgrades now fail closed exactly like fresh installs; `DD_ANONYMOUS_AUTH_CONFIRM=true` is the opt-in. - **Session cookie rename (A4)**: `connect.sid` → `drydock.sid` one-time sign-out documented in DEPRECATIONS.md, the upgrading section, and the README banner. - **SSRF hardening (A2)**: the HTTP trigger page now documents the guarded DNS lookup, redirect refusal, and the `ALLOWMETADATA` escape hatch. - **WebSocket origin checks (A3)**: server config, FAQ, quickstart reverse-proxy warning, `api/log.mdx`, and `api/container.mdx` now state that origin validation covers scheme/host/port and that forwarded headers are honored only with `DD_SERVER_TRUSTPROXY`. - **UI vocabulary (B3/B4/B5)**: watchers page and FAQ updated for the pin-glyph/"Current" pinned treatment; eligibility + API docs document the new additive `clockSource`/`clockStartAt` blocker fields; the UI page covers the column-picker auto-hidden annotation. Wording grounded in the actual #566 locale strings, not paraphrased. - **Website quickstarts actually start**: the shared `DockerRunSnippet` (all 7 comparison pages) and the FAQ quickstart now include the auth opt-in env var — without it the one-liner produces a container that refuses to start on the v1.6 line. Also rides along: `protobufjs` pinned to 7.6.5 (CVE-2026-59877 / GHSA-j3f2-48v5-ccww, third fresh advisory today) in app (new override) and e2e (existing override bumped) — it began failing the qlty osv-scanner gate mid-push. Deliberately skipped: `content/docs/current/updates/index.mdx` (strictly per-release curated notes; rc.3 highlights belong to the actual cut) and `UPGRADE-NOTES.md` (version-gated to 1.4.6–1.5.x by `scripts/append-upgrade-notes.mjs`; v1.6 content there would leak into 1.5.x release notes). ## Why The security batch merged with zero docs changes, leaving the auth docs actively contradicting shipped behavior. Docs on `dev/v1.6` publish at the GA dev→main merge, so these corrections go live exactly when the behavior does. ## Tests Full pre-push gate green. apps/web script tests 43/43; MDX parse validated via sync-docs + fumadocs-mdx; biome clean. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Changelog - 🔒 Documented fail-closed anonymous authentication for both fresh installs and upgrades; require `DD_ANONYMOUS_AUTH_CONFIRM=true` (and clarified `/health` vs API response behavior). -⚠️ Security documented “no deprecation window” enforcement: removed upgrade grandfather path when anonymous auth isn’t configured/confirmed; upgrades stop serving/fail closed. -⚠️ 🔒 Documented `connect.sid` → `drydock.sid` session cookie rename and one-time sign-out on upgrade. - 🔒 Documented SSRF protections for guarded DNS lookup, redirect refusal (3xx never followed), and `ALLOWMETADATA` escape hatch. - 🔒 Documented WebSocket `Origin` validation (scheme/host/port) and clarified reverse-proxy requirements via `DD_SERVER_TRUSTPROXY` + forwarded headers (`X-Forwarded-Proto`/`X-Forwarded-Host`), including log-stream connection failures when misconfigured. - ✨ Documented maturity-clock details for `maturity-not-reached` (clock source + ISO-8601 start timestamp payload) and UI countdown rendering behavior. - ✨ Updated UI copy/annotations for column visibility and “hidden to fit” behavior. - ✨ Updated watcher UI description for fully-pinned tags to reflect `updateInsight` chip styling + persistent pin glyph/tooltips. - 🔧 Added `DD_ANONYMOUS_AUTH_CONFIRM=true` to website quickstarts and updated Docker run snippet wording. - 🔧 Pinned `protobufjs` to `7.6.5` in both app and e2e dependencies. - 🔧 Updated doc identity/release tests to assert current fail-closed semantics for `content/docs/current` while preserving legacy v1.5 behavior. ## Concerns - Confirm `DD_SERVER_TRUSTPROXY` docs explicitly require forwarding `X-Forwarded-Proto` and `X-Forwarded-Host` for scheme/host matching. - Validate that the documented fail-closed behavior matches the actual endpoints: API 401 behavior vs `/health` 503 reporting. - Ensure the “redirects never followed” documentation aligns with the actual redirect-following disablement and how `ALLOWMETADATA` is evaluated. - Verify cookie rename handling is documented consistently across all relevant upgrade/install docs (including one-time sign-out semantics). - Check that WebSocket origin validation requirements cover both dashboard log viewer and container log streaming endpoints (upgrade vs steady-state behavior). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
scttbnsn
added a commit
that referenced
this pull request
Jul 21, 2026
Release plumbing for the v1.6.0-rc.3 cut — the full identity bump the guard tests enforce. - Dated `[1.6.0-rc.3] — 2026-07-21` CHANGELOG heading, fresh empty `[Unreleased]` section, matching link-reference definitions - rc.3 across README badge/highlights summary, website site-config + timeline, updates page (new rc.3 highlights section), API doc example payloads, quickstart tag matrix, and the demo runtime fixtures - Guard tests updated in lockstep: release-identity, release-docs-identity (compare base now `v1.6.0-rc.2...v1.6.0-rc.3`), changelog-links rc.3 delta over rc.2: #566 display-honesty batch, #567 security hardening, #568 maturity-clock restart fix (#565), #569 docs freshness. Verified: 105/105 script tests, 43/43 web-script tests, full pre-push gate green. Last dev-side change before the release head freeze. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Changelog 🔧 **Changed** - Updated release identity from `v1.6.0-rc.2` to `v1.6.0-rc.3` across README, website metadata, roadmap, docs, API examples, quickstart tags, and demo fixtures. - Added the dated `v1.6.0-rc.3` updates section and refreshed changelog link references. - Updated release identity and documentation guard tests, including rc comparison ranges and candidate fixtures. ✨ **Added** - Documented the rc.3 changes covering issues `#566`, `#567`, `#568/`#565, and `#569`. 🔒 **Security** - Documented the rc.3 security hardening batch. ## Verification - Script tests: 105/105 passed - Web-script tests: 43/43 passed - Full pre-push gate: passed ## Concerns - Verify the release date `July 21, 2026` is correct. - Confirm all remaining `1.6.0-rc.2` references are intentional. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Merged
scttbnsn
added a commit
that referenced
this pull request
Jul 21, 2026
Promotes the v1.6.0-rc.3 content from `dev/v1.6` to `main` for the release cut. rc.3 delta over rc.2: - **#566 display-honesty batch** — update-status vocabulary ("Digest update", Unknown badge, "Security hold"), pin glyph driven by the real pin-gate verdict (`tagPinGated`), insight-only pinned rows read Current everywhere (table + detail panel), one-clock maturity panel, column-picker auto-hide annotations, registries deep-link fix (#556), banner pluralization, tooltip freshness. - **#567 security hardening** — anonymous auth fails closed on upgrades, HTTP trigger SSRF guards, full WebSocket origin validation, `drydock.sid` session cookie, owner-only store perms, pinned icon CDN revisions, CI secret scans. - **#568 maturity-clock restart fix** (#565) — the soak clock restarts only when the update candidate identity changes. - **#569 docs freshness** — auth/SSRF/WS/UI copy corrected to shipped v1.6 behavior. - **#571 release identity bump** — dated CHANGELOG heading, rc.3 across all public version surfaces, guard tests in lockstep. After merge: dispatch `release-cut.yml` from `main` with `release_tag=v1.6.0-rc.3`.
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
1 participant
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.
What
Nine security-hardening findings from the 2026-07-20 internal security review, as 12 atomic commits plus CHANGELOG entries:
app/triggers/providers/http, Hass webhook URL handling, registry/icon fetches): guarded DNS lookup blocks cloud-metadata/link-local resolution on every request (override:allowmetadata=true), and redirects can't hop into blocked address space (DNS-rebinding + cross-host redirect protection).app/api/ws-upgrade-utils.ts): full scheme/host/port origin comparison;X-Forwarded-Proto/X-Forwarded-Hosthonored only with trust proxy enabled; socket transport used otherwise.app/authentications/providers/anonymous): the warn-and-serve grandfather path is gone — no auth configured (or anonymous unconfirmed) now refuses to start, same as fresh installs.DD_ANONYMOUS_AUTH_CONFIRM=trueis the explicit opt-in. Behavior change for upgraders running open dashboards — called out in the CHANGELOG./storecreated0700(Dockerfile),dd.json0600,umask 077in the entrypoint and store process so Loki autosave temp/replacement files never widen.drydock.sidreplaces the Express defaultconnect.sid. One-time sign-out on upgrade.app/api/icons/providers.ts): dashboard-icons/selfh.st/simple-icons pinned to exact revisions/versions instead of floating refs.apps/web): static'unsafe-inline'header CSP replaced by a request-scoped nonce CSP built insrc/proxy.tsper Next.js guidance;experimental.sristays off (the existing guard test still enforces that) and the vercel.json CSP header is removed in favor of the nonce policy. Newcontent-security-policy.test.mjscovers the builder.ci-verify.ymlwith an exact-baseline.gitleaksignore(427 historical entries) andscripts/scan-secrets.shfor local parity.Why
Internal security review follow-up (report kept local by policy). Targets the v1.6.1/rc.3 train via
dev/v1.6— nothing here touches the rc.2 soak or the GA promotion candidate.Tests
Full pre-push gate green on this exact branch (cut from
origin/dev/v1.6, cherry-picked 1:1 from the review line): app and ui coverage gates at 100% on all four metrics, builds, biome, qlty, workflow tests. New coverage: forwarded-protocol WS branches, fail-closed upgrade auth, SSRF lookup guards, store-permission enforcement, CSP builder.Notes for review
vercel.jsonloses its static CSP intentionally — the nonce policy fromproxy.tssupersedes it (a static header can't carry a per-request nonce).Changelog
maxRedirects: 0).Originvalidation by checking both effective protocol + host with TLS/trust-proxy awareness.DD_ANONYMOUS_AUTH_CONFIRM=trueand fails closed during upgrade without confirmation.drydock.sid, updating CSRF and OpenAPI security scheme configuration and related tests./storepermissions (0700), and Docker entrypoint-created files viaumask 077; store permission repair now avoids the TOCTOU window by handlingENOENTonly.JsonLdcomponent.Concerns
.gitleaksignorebaseline may be brittle as repo files evolve—ensure it stays aligned with the pinned scanning commands and modes.drydock.sidcookie rename (including any external reverse proxies or browser automation).all: truelookups, and connection-time resolution failures; ensure metadata/link-local detection is complete for every resolved address.ENOENTswallowed; all other errors rethrown) and thatumaskis correctly captured/restored.