diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e15853e..d80889bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -222,25 +222,24 @@ jobs: - name: Run end-to-end smoke test run: | - # This profile intentionally has HIGH security findings against it - # in CI (env-derived DB passwords look like hardcoded secrets to - # CDS-SEC-001/003/041, and cosign is not installed on this runner, - # so CDS-VER-001 also fires) — the same reason the "Verify - # unverifiable images fail closed" step below expects `cds - # security` to exit 1. `cds test` bundles security into its single - # exit code, so assert only that validate/plan/render passed and - # that the sole failing stage is "security", rather than requiring - # the whole command to exit 0. + # With the env-derived secret values set above, the security stage + # passes: since #979 the renderer references in initDbEnv.* and + # the waiver reason no longer false-positive as hardcoded secrets, + # and only the known MEDIUM gaps (CDS-SEC-080/081) remain, which + # do not fail the stage. Image verification is separate: cosign is + # not installed on this runner, so the "Verify unverifiable images + # fail closed" step below still expects `cds security + # --verify-images` to exit 1 with CDS-VER-* findings. set +e test_output="$(cds test local-dagster-postgres-superset 2>&1)" test_status=$? set -e echo "$test_output" grep -Eq "^\[PASS\] validate$" <<<"$test_output" - grep -Eq "^\[FAIL\] security$" <<<"$test_output" + grep -Eq "^\[PASS\] security$" <<<"$test_output" grep -Eq "^\[PASS\] plan$" <<<"$test_output" grep -Eq "^\[PASS\] render$" <<<"$test_output" - test "$test_status" -eq 1 + test "$test_status" -eq 0 - name: Generate MVP plan run: cds plan local-dagster-postgres-superset --output /tmp/plan.json diff --git a/CHANGELOG.md b/CHANGELOG.md index d71cb048..2f01ea43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,21 @@ The format is based on Keep a Changelog. ### Fixed +- Fixed `cds security` false positives on the reference production profile + (#979): the secret-value rules CDS-SEC-001/003/033 no longer flag values + that are entirely a renderer reference (`config.passwordFrom`, + `${secrets.x}`, ...), which the declarative engine sees unresolved in the + planned config but the renderer resolves to `${CDS_*}` placeholders at + render time (new opt-in `ignoreRendererReferences` rule flag, rule set + 1.0.1); the CDS-SEC-013 secret-reuse check skips such references as well; + the waiver-reason entropy exemption now also matches the flattened + `security.waivers.*.reason` path (spec-level keys flatten without the + `spec.` prefix); and CDS-SEC-031 matches the env file path relative to the + project root (the same marker walk `resolve_env_file_path` uses, now shared + via `cli/security_common.py`), so a bare project-root `.env` never fires + when the CLI is invoked from another directory (e.g. with + `CDS_PROFILE_PATH` set). + - Fixed the `helm` render target lacking the provide-side contract-host cross-check that the compose target got for issue #1010: the Kubernetes renderer only validated the consume side (E084), so a provided diff --git a/cli/main.py b/cli/main.py index 5ea4cdbb..b1bc1114 100644 --- a/cli/main.py +++ b/cli/main.py @@ -62,6 +62,7 @@ NIS2_ARTICLE_21_MEASURES, SEVERITY_ORDER, infer_profile_class, + resolve_project_root, ) from .state import format_state_output, group_services_by_health, parse_compose_ps_json from .up_runner import ( @@ -738,27 +739,6 @@ def resolve_profile_path(profile: str | None) -> str: ) -def resolve_project_root(profile_path: str) -> Path: - """ - Resolve a project root for output artifacts. - - The resolver walks up from the selected profile location and picks the - first directory containing `.cds` (CDS's own state directory, checked - first so it takes priority over an unrelated ancestor repository), - `pyproject.toml`, or `.git`. If no marker is found, it falls back to the - current working directory. - """ - start = Path(profile_path).resolve().parent - for directory in [start, *start.parents]: - if ( - (directory / ".cds").exists() - or (directory / "pyproject.toml").exists() - or (directory / ".git").exists() - ): - return directory - return Path.cwd().resolve() - - def resolve_env_file_path(profile_path: str) -> Path: """ Resolve the default .env location for a profile. diff --git a/cli/resources/rule-schema.json b/cli/resources/rule-schema.json index eee81ff3..da2369b5 100644 --- a/cli/resources/rule-schema.json +++ b/cli/resources/rule-schema.json @@ -253,6 +253,11 @@ "type": "boolean", "default": true }, + "ignoreRendererReferences": { + "type": "boolean", + "default": false, + "description": "For rules that match on secret-like *values*: skip values that are entirely a renderer reference expression (bare 'config.x'/'secrets.x'/'bindings.x'/'service.x'/'certificates.x' or the ${...}-wrapped form). The declarative engine scans the planned config where those references are still unresolved strings, but the renderer resolves them before anything is executed (secrets.*/certificates.* become ${CDS_*} Compose placeholders), so they are pointers, not literal secrets (#979)." + }, "codeEnforced": { "type": "boolean", "default": true, diff --git a/cli/resources/rule-set.json b/cli/resources/rule-set.json index c8520bed..d7a326b5 100644 --- a/cli/resources/rule-set.json +++ b/cli/resources/rule-set.json @@ -1,6 +1,6 @@ { "$schema": "rule-schema.json", - "version": "1.0.0", + "version": "1.0.1", "metadata": { "name": "cds-core-security-rules", "description": "Core security validation rules for cds validate --security", @@ -30,7 +30,8 @@ "Move the value to environment variables or a secret backend.", "Replace literal values with references." ], - "enabled": true + "enabled": true, + "ignoreRendererReferences": true }, { "id": "CDS-SEC-002", @@ -81,7 +82,8 @@ "Store private keys in a secret backend or mounted secret file.", "Do not keep cryptographic material directly in profile YAML." ], - "enabled": true + "enabled": true, + "ignoreRendererReferences": true }, { "id": "CDS-SEC-004", @@ -439,7 +441,7 @@ }, { "id": "CDS-SEC-031", - "$comment": "Only the project-root .env is covered by this repo's `/.env` .gitignore entry (leading slash pins it to the root). Any nested .env (profile-scoped, rendered/generated output, or otherwise) is NOT covered by that ignore rule and can be committed accidentally, so the pathPatterns below only flag paths with at least one directory component before the filename; a bare root '.env' is the expected, already-ignored location and must not fire here.", + "$comment": "Only the project-root .env is covered by this repo's `/.env` .gitignore entry (leading slash pins it to the root). Any nested .env (profile-scoped, rendered/generated output, or otherwise) is NOT covered by that ignore rule and can be committed accidentally, so the pathPatterns below only flag paths with at least one directory component before the filename; a bare root '.env' is the expected, already-ignored location and must not fire here. The engine matches the env file path relative to the project root (resolved from the profile path, same marker walk as resolve_env_file_path), not relative to the invocation cwd, so an absolute project-root .env (e.g. when the CLI runs from another directory with CDS_PROFILE_PATH set) is still seen as the bare '.env' and never matches (#979).", "title": "Env file path appears repository-managed or unmanaged", "severity": "medium", "category": "file-output-safety", @@ -512,7 +514,8 @@ "Use environment or secret-backend references for sensitive fields.", "Keep profiles declarative and secret-free." ], - "enabled": true + "enabled": true, + "ignoreRendererReferences": true }, { "id": "CDS-SEC-040", diff --git a/cli/security.py b/cli/security.py index bb1a3ed2..f5c453dd 100644 --- a/cli/security.py +++ b/cli/security.py @@ -30,7 +30,12 @@ resolved_secret_values, ) from .secrets import load_secrets_from_env -from .security_common import SECRET_KEY_RE, SEVERITY_ORDER, infer_profile_class +from .security_common import ( + SECRET_KEY_RE, + SEVERITY_ORDER, + infer_profile_class, + resolve_project_root, +) from .utils import _compose_service_name _PROFILE_SCOPES = { @@ -192,29 +197,42 @@ def _flatten_env_secrets(secrets: dict[str, str]) -> list[tuple[str, str, Any]]: return [("", f"secrets.{key}", value) for key, value in secrets.items()] -def _normalize_scan_path(path: Path) -> str: +def _normalize_scan_path(path: Path, project_root: Path | None = None) -> str: + """ + Render a scanned file path relative to the project root when possible, so + the project-root `.env` is always the bare ".env" no matter which directory + the CLI was invoked from (e.g. with CDS_PROFILE_PATH set); fall back to a + cwd-relative path, then to the absolute path. + """ resolved = path.resolve() - try: - return resolved.relative_to(Path.cwd().resolve()).as_posix() - except ValueError: - return resolved.as_posix() + for base in (project_root, Path.cwd().resolve()): + if base is None: + continue + try: + return resolved.relative_to(base).as_posix() + except ValueError: + continue + return resolved.as_posix() def _flatten_env_inputs( secrets: dict[str, str], env_file: str | None, + project_root: Path | None = None, ) -> list[tuple[str, str, Any]]: """ Emit both loaded .env secrets and the env file path itself when present. Some rules evaluate how a secret-bearing env file is located or managed rather than inspecting individual secret values. Represent the file path as - a synthetic flat item so those rules can use the same matcher. + a synthetic flat item so those rules can use the same matcher. The path is + normalized against `project_root` (not just cwd) so a bare project-root + `.env` stays bare regardless of the invocation directory. """ items = _flatten_env_secrets(secrets) env_path = Path(env_file) if env_file is not None else Path(".env") if env_path.exists(): - scan_path = _normalize_scan_path(env_path) + scan_path = _normalize_scan_path(env_path, project_root) items.append(("", scan_path, scan_path)) return items @@ -223,7 +241,10 @@ def _is_windows() -> bool: return os.name == "nt" -def _flatten_file_permissions(env_file: str | None) -> list[tuple[str, str, Any]]: +def _flatten_file_permissions( + env_file: str | None, + project_root: Path | None = None, +) -> list[tuple[str, str, Any]]: """ Emit the POSIX permission bits of the secret-bearing .env file as a synthetic item whose value is the octal mode string (e.g. "0644"). @@ -240,7 +261,7 @@ def _flatten_file_permissions(env_file: str | None) -> list[tuple[str, str, Any] mode = stat.S_IMODE(env_path.stat().st_mode) except OSError: return [] - return [("", f"{_normalize_scan_path(env_path)}.permissions", f"{mode:04o}")] + return [("", f"{_normalize_scan_path(env_path, project_root)}.permissions", f"{mode:04o}")] def _permission_mode(value: str) -> int | None: @@ -329,6 +350,28 @@ def _is_secret_reference(value: Any) -> bool: return isinstance(value, str) and bool(_SECRET_REF_RE.match(value)) +# The renderer's reference vocabulary (cli/renderer.py's _resolve_expr), in +# both its bare form (e.g. "config.passwordFrom", "secrets.cache_password", +# used in module config positions the renderer resolves, such as initDbEnv) +# and its ${...}-wrapped form (e.g. "${config.passwordFrom}"). A value that +# is entirely such an expression is a pointer the renderer resolves at render +# time (secrets.*/certificates.* become ${CDS_*} Compose placeholders), never +# a literal secret, so secret-value rules that opt in via the rule's +# `ignoreRendererReferences` flag skip it (#979). +_RENDERER_REF_RE = re.compile( + r"^\$\{(?:secrets|certificates|config|bindings|service)\.[^}]+\}$" # ${config.x} + r"|^(?:secrets|certificates|config|bindings|service)\.[A-Za-z0-9_.]+$" # config.x +) + +def _is_renderer_reference(value: Any) -> bool: + """ + Returns True if value is entirely a renderer reference expression + (bare "config.x"/"secrets.x"/... or "${config.x}"/"${secrets.x}"/...), + i.e. a value the renderer resolves at render time rather than a literal. + """ + return isinstance(value, str) and bool(_RENDERER_REF_RE.match(value)) + + # --------------------------------------------------------------------------- # Profile class inference # --------------------------------------------------------------------------- @@ -394,9 +437,13 @@ def _redact(value: Any) -> str | None: # secret heuristic. Unlike _NON_SECRET_PATH_SUFFIXES, these only suppress the # check for the exact documented field, so an unrelated key that happens to # end in "reason" (e.g. a genuine secret named "authFailureReason") is still -# scanned normally. +# scanned normally. Both the "spec."-prefixed and the bare form are listed: +# _flatten_profile_by_module emits spec-level keys without the "spec." prefix +# (e.g. "security.waivers.runtimePolicy.reason"), while callers that flatten +# the raw profile document use the prefixed form. _NON_SECRET_PATH_PATTERNS = ( "spec.security.waivers.*.reason", + "security.waivers.*.reason", ) def _eval_condition( path: str, @@ -498,6 +545,12 @@ def _check_secret_reuse( for module_id, path, value in flat_items: if not isinstance(value, str) or not value: continue + # Renderer references (e.g. "secrets.backup_encryption_key") are + # pointers resolved at render time, not secret values: sharing one + # reference across fields is the intended indirection, not reuse of + # the underlying value (#979). + if _is_renderer_reference(value): + continue if not SECRET_KEY_RE.search(path.split(".")[-1]): continue value_to_locations.setdefault(value, []).append((module_id, path)) @@ -535,12 +588,20 @@ def _rule_matches( findings: list[dict[str, Any]] = [] match = rule["match"] + # Rules flagging secret-like *values* can opt out of renderer references + # (bare "config.x" or "${secrets.x}" expressions): the declarative engine + # scans the planned config where those references are still unresolved + # strings, but the renderer resolves them before anything is executed, + # and rendered output only ever contains ${CDS_*} placeholders (#979). + skip_renderer_refs = bool(rule.get("ignoreRendererReferences", False)) for module_id, path, value in flat_items: # Skip unresolved ${secrets.*} references in profile config. # They are intentional indirections, not real values. if _is_secret_reference(value): continue + if skip_renderer_refs and _is_renderer_reference(value): + continue key = path.split(".")[-1] if path else "" @@ -1382,8 +1443,13 @@ def run_security_validation( secrets, secret_diags = load_secrets_from_env(env_file) flat_profile = _flatten_profile_by_module(profile, profile_dir=profile_path.parent) - flat_env = _flatten_env_inputs(secrets, env_file) - flat_permissions = _flatten_file_permissions(env_file) + # Normalize scanned file paths (e.g. the .env path CDS-SEC-031 matches on) + # against the same project root resolve_env_file_path uses, so a bare + # project-root .env never matches regardless of the invocation directory + # (e.g. when CDS_PROFILE_PATH points at a profile while cwd is elsewhere). + scan_project_root = resolve_project_root(str(profile_path)) + flat_env = _flatten_env_inputs(secrets, env_file, project_root=scan_project_root) + flat_permissions = _flatten_file_permissions(env_file, project_root=scan_project_root) # Planning and rendering the profile is only useful when some enabled # rule actually declares the "rendered-compose" scope -- e.g. a custom diff --git a/cli/security_common.py b/cli/security_common.py index aaa0a379..ea937c3e 100644 --- a/cli/security_common.py +++ b/cli/security_common.py @@ -1,10 +1,32 @@ from __future__ import annotations import re +from pathlib import Path from typing import Any SEVERITY_ORDER = {"high": 0, "medium": 1, "low": 2} + +def resolve_project_root(profile_path: str) -> Path: + """ + Resolve a project root for output artifacts. + + The resolver walks up from the selected profile location and picks the + first directory containing `.cds` (CDS's own state directory, checked + first so it takes priority over an unrelated ancestor repository), + `pyproject.toml`, or `.git`. If no marker is found, it falls back to the + current working directory. + """ + start = Path(profile_path).resolve().parent + for directory in [start, *start.parents]: + if ( + (directory / ".cds").exists() + or (directory / "pyproject.toml").exists() + or (directory / ".git").exists() + ): + return directory + return Path.cwd().resolve() + # Closed set of informational compliance control categories a rule-set.json # rule can be tagged with (rule-schema.json's $defs.complianceCategory), # each mapped to a short display label plus the rationale documented in diff --git a/docs/production-hardening-checklist.md b/docs/production-hardening-checklist.md index f568cb69..2b2fd060 100644 --- a/docs/production-hardening-checklist.md +++ b/docs/production-hardening-checklist.md @@ -178,19 +178,27 @@ Expected: `cds state` shows every service healthy; the proxy's TLS entry points answer (for example `curl -v https:///`) and the backend ports answer only through the proxy. `cds down` stops the stack. -## 4. Expected findings (snapshot 2026-10-26) +## 4. Expected findings (snapshot 2026-10-30) `cds security team-dagster-postgres-superset --environment prod` currently exits 1 with the following findings. The set is stable; a deviation in either direction is the operator's signal to stop and investigate. +Renderer references (for example `config.passwordFrom` in `initDbEnv.*`, +`${secrets.*}` aliases) and the free-text waiver reason are no longer +flagged: since #979 the secret-value rules CDS-SEC-001/003/033 skip values +that are entirely a renderer reference (new `ignoreRendererReferences` rule +flag), the CDS-SEC-013 reuse check skips such references, the waiver-reason +entropy exemption covers the flattened `security.waivers.*.reason` path, and +CDS-SEC-031 matches the env file path relative to the project root so a bare +project-root `.env` never fires regardless of the invocation directory. +Rendered output still contains only `${CDS_*}` placeholders. + | Finding | Count | Classification | Action / tracking | | --- | --- | --- | --- | -| `CDS-SEC-001`/`CDS-SEC-033` on `initDbEnv.*`; `CDS-SEC-003` on `initDbEnv.*` and on the waiver reason | 11 | False positive: the flagged values are renderer references (for example `config.passwordFrom`) or free-text waiver reasons, not secrets; rendered output contains only `${CDS_*}` placeholders | [#979](https://github.com/RonaldHensbergen/composable-data-stack/issues/979) | | `CDS-SEC-074` on `services.dagster-webserver.ports[0]` and `services.superset.ports[0]` | 2 | True positive: the admin backends still publish directly to the host, allowing a proxy bypass | Remove the backend host publishes (proxy becomes the sole entry point), or set `spec.security.waivers.plaintextEndpointExposure.reason` | | `CDS-SEC-081` on the admin-facing services | 3 | Known gap: no identity module is bound | [#370](https://github.com/RonaldHensbergen/composable-data-stack/issues/370) | | `CDS-SEC-080` on `spec.modules` | 1 | Known gap: the backup policy is declared at profile level (`spec.backup`), which the rule does not yet recognize as contract consumption | [backup-restore.md](backup-restore.md) follow-ups ([#210](https://github.com/RonaldHensbergen/composable-data-stack/issues/210)) | -| `CDS-SEC-013` on `backup.encryption.keyFrom` / `backup.encryptionKeyFrom` | 2 | Accepted: one backup encryption key is deliberately referenced by both the module capability and the profile policy | None (single purpose, single key) | | `W103` on `spec.security.waivers.runtimePolicy` | 1 | Informational: the documented non-root waiver for images that set `USER` in their Dockerfiles | Keep the waiver reason accurate | Additional `CDS-SEC-004` (placeholder/weak secret) or `CDS-SEC-012` (too diff --git a/tests/test_cds_workflow.py b/tests/test_cds_workflow.py index 2c19f4fc..b117a332 100644 --- a/tests/test_cds_workflow.py +++ b/tests/test_cds_workflow.py @@ -91,12 +91,13 @@ def test_commands(self): def test_test_command(self): """`cds test` bundles validate/security/plan/render into one exit code. - This profile's committed secret values (from `cds init`) are the - same kind of low-entropy, non-external-reference values CI supplies - via env vars, so the security stage always reports HIGH findings - (CDS-SEC-001/003) here -- the same reason CI's "Run end-to-end smoke - test" step (see .github/workflows/ci.yml) asserts this specific - per-stage outcome instead of requiring `cds test` to exit 0. + The init'd project's committed `.env` holds generated placeholder + secret values, so the security stage always reports HIGH findings + (CDS-SEC-012 weak/predictable values on those literals) here and + `cds test` exits 1. CI's "Run end-to-end smoke test" step + (.github/workflows/ci.yml) instead supplies env vars and, since + #979 removed the renderer-reference false positives, expects every + stage to pass there. """ env_with_profile = {"CDS_PROFILE_PATH": self.profile_file} diff --git a/tests/test_security.py b/tests/test_security.py index 55f7afa5..deef2163 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -13,9 +13,11 @@ _check_admin_service_identity_binding, _check_backup_target_binding, _check_production_plaintext_exposure, + _check_secret_reuse, _eval_condition, _flatten_profile_by_module, _flatten_rendered_leak_surfaces, + _is_renderer_reference, _map_service_to_module, _module_provides_plaintext_http, _module_provides_tls_reverse_proxy, @@ -24,6 +26,7 @@ _plaintext_module_ids_fronted_by_tls_reverse_proxy, _port_is_non_local_host_exposure, _redact, + _rule_matches, _try_render_compose_for_scan, _validate_rule_set, run_security_validation, @@ -39,7 +42,7 @@ class BundledSecurityRulesTest(unittest.TestCase): def test_default_rule_set_is_available_as_package_data(self): rule_set = _validate_rule_set() - self.assertEqual(rule_set["version"], "1.0.0") + self.assertEqual(rule_set["version"], "1.0.1") self.assertGreater(len(rule_set["rules"]), 0) def test_invalid_rule_set_raises_validation_error_with_paths(self): @@ -464,6 +467,217 @@ def test_does_not_flag_the_conventional_project_root_env_file(self): hits = [f for f in findings if f["rule_id"] == "CDS-SEC-031"] self.assertEqual(hits, [], "a bare project-root .env must not be flagged") + def _scan_env_from_outside_project(self, env_rel: str) -> list[dict]: + """Build a throwaway project (`.git` marker + minimal profile + an + .env at `env_rel`), then run the scan from an unrelated cwd with the + env file passed as an absolute path — the CDS_PROFILE_PATH scenario + from #979. Returns the full findings list.""" + with tempfile.TemporaryDirectory() as project_dir, tempfile.TemporaryDirectory() as other_dir: + project_root = Path(project_dir) + (project_root / ".git").mkdir() # project-root marker + profile_path = project_root / "profile.yaml" + profile_path.write_text( + "apiVersion: cds/v1alpha1\nkind: Profile\n" + "metadata:\n name: tmp\n environment: local\n" + "spec:\n modules: []\n", + encoding="utf-8", + ) + env_file = project_root / env_rel + env_file.parent.mkdir(parents=True, exist_ok=True) + env_file.write_text("SOME_VAR=value\n", encoding="utf-8") + + original_cwd = Path.cwd() + try: + os.chdir(other_dir) + findings, _diags = run_security_validation( + profile_path, + _RULE_SCHEMA_PATH, + _RULE_SET_PATH, + env_file=str(env_file), + ) + finally: + os.chdir(original_cwd) + return findings + + def test_root_env_file_not_flagged_when_invoked_outside_project_root(self): + """#979: with cwd outside the project (e.g. CDS_PROFILE_PATH set), the + project-root .env arrives as an absolute path; matching it relative to + the project root keeps it the bare '.env', which must not fire.""" + findings = self._scan_env_from_outside_project(".env") + hits = [f for f in findings if f["rule_id"] == "CDS-SEC-031"] + self.assertEqual( + hits, [], + "a project-root .env must not be flagged even when the CLI runs from another directory", + ) + + def test_nested_env_file_flagged_relative_to_project_root(self): + """#979: a nested .env is still flagged, with the path shown relative + to the project root rather than to the invocation cwd.""" + findings = self._scan_env_from_outside_project("nested/.env") + hits = [f for f in findings if f["rule_id"] == "CDS-SEC-031"] + self.assertEqual(len(hits), 1) + self.assertEqual(hits[0]["path"], "nested/.env") + + +class RendererReferenceDetectionTest(unittest.TestCase): + def test_bare_and_wrapped_reference_forms_are_references(self): + for value in ( + "config.passwordFrom", + "config.dagsterDatabase.username", + "secrets.cache_password", + "bindings.db.host", + "service.host", + "certificates.web.cert", + "${config.passwordFrom}", + "${secrets.cache_password}", + "${bindings.db.host}", + ): + self.assertTrue(_is_renderer_reference(value), value) + + def test_literals_placeholders_and_mixed_strings_are_not_references(self): + for value in ( + "${CDS_ANALYTICS_DB_NAME}", + "db://${bindings.db.host}:5432", + "Sup3r-Secret-Literal-2026!", + "example", + "", + 123, + None, + ): + self.assertFalse(_is_renderer_reference(value), value) + + +class RendererReferenceSecretRuleTest(unittest.TestCase): + """#979: secret-value rules must not flag renderer references, which the + declarative engine sees unresolved in the planned config but the renderer + resolves (to ${CDS_*} placeholders) before anything executes.""" + + def _rule(self, rule_id): + return next(r for r in _validate_rule_set()["rules"] if r["id"] == rule_id) + + def test_reference_flagged_rules_skip_renderer_references(self): + items = [ + ("postgres", "initDbEnv.ANALYTICS_DB_PASSWORD", "config.passwordFrom"), + ("postgres", "initDbEnv.DAGSTER_DB_PASSWORD", "config.dagsterDatabase.passwordFrom"), + ("postgres", "initDbEnv.DAGSTER_DB_USER", "config.dagsterDatabase.username"), + ("postgres", "initDbEnv.SUPERSET_DB_USER", "config.supersetDatabase.username"), + ("postgres", "passwordFrom", "secrets.analytics_db_password"), + ("postgres", "secretKeyFrom", "${secrets.superset_secret_key}"), + ] + for rule_id in ("CDS-SEC-001", "CDS-SEC-003", "CDS-SEC-033"): + rule = self._rule(rule_id) + self.assertTrue(rule.get("ignoreRendererReferences"), rule_id) + self.assertEqual( + _rule_matches(rule, items, "prod"), [], + f"{rule_id} must not fire on renderer references", + ) + + def test_secret_value_rules_still_flag_literal_secrets(self): + items = [ + ("postgres", "initDbEnv.ANALYTICS_DB_PASSWORD", "Sup3r-Secret-Literal-2026!x"), + ("postgres", "passwordFrom", "hunter2-literal"), + ] + for rule_id in ("CDS-SEC-001", "CDS-SEC-003", "CDS-SEC-033"): + findings = _rule_matches(self._rule(rule_id), items, "prod") + self.assertTrue(findings, f"{rule_id} must still fire on literal secret values") + + def test_rules_without_the_flag_do_not_skip_references(self): + # The skip is an explicit per-rule opt-in: a rule that does not set + # ignoreRendererReferences still sees the raw reference value. + rule = { + "id": "CDS-SEC-999", + "severity": "low", + "match": {"any": [{"valueRegex": r"^config\."}]}, + "message": "m", + "recommendation": [], + } + items = [("postgres", "initDbEnv.X", "config.passwordFrom")] + self.assertEqual(len(_rule_matches(rule, items, "prod")), 1) + + def test_secret_reuse_check_skips_renderer_references(self): + items = [ + ("", "backup.encryption.keyFrom", "secrets.backup_encryption_key"), + ("postgres", "backup.encryptionKeyFrom", "secrets.backup_encryption_key"), + ] + self.assertEqual(_check_secret_reuse(items), []) + + def test_secret_reuse_check_still_flags_reused_literal_values(self): + items = [ + ("", "secrets.CDS_A_PASSWORD", "Shared-Literal-Value-2026!"), + ("", "secrets.CDS_B_PASSWORD", "Shared-Literal-Value-2026!"), + ] + findings = _check_secret_reuse(items) + self.assertEqual(len(findings), 2) + self.assertTrue(all(f["rule_id"] == "CDS-SEC-013" for f in findings)) + + +class WaiverReasonFlattenedPathTest(unittest.TestCase): + def test_entropy_rule_ignores_unprefixed_waiver_reason_path(self): + # _flatten_profile_by_module emits spec-level keys without the + # "spec." prefix, so the waiver reason flattens to + # "security.waivers.runtimePolicy.reason" (#979). + matched = _eval_condition( + path="security.waivers.runtimePolicy.reason", + key="reason", + value="Dagster and Superset images set a non-root USER in their Dockerfiles.", + cond={"entropy": "high", "minLength": 24}, + profile_class="prod", + ) + self.assertFalse(matched) + + +class ReferenceProfileRegressionTest(unittest.TestCase): + """#979 end-to-end: the reference production profile's initDbEnv + references, waiver reason, and shared key references produce no + CDS-SEC-001/003/013/033 findings.""" + + _ENV = "\n".join( + f"{k}={v}" + for k, v in { + "CDS_POSTGRES_SUPERUSER_PASSWORD": "Str0ng!Superuser#2026x", + "CDS_ANALYTICS_DB_PASSWORD": "An4lytics!Pass#2026x", + "CDS_DAGSTER_DB_PASSWORD": "D4gster!Pass#2026x", + "CDS_SUPERSET_DB_PASSWORD": "Sup3rset!Pass#2026x", + "CDS_SUPERSET_SECRET_KEY": "Sup3rsetSecretKey!2026x", + "CDS_SUPERSET_ADMIN_PASSWORD": "Adm1n!Pass#2026x", + "CDS_CACHE_PASSWORD": "C4che!Pass#2026x", + "CDS_BACKUP_ENCRYPTION_KEY": "B4ckup!Key#2026x", + "CDS_ANALYTICS_DB_NAME": "analytics", + "CDS_ANALYTICS_DB_USER": "analytics_user", + "CDS_DAGSTER_DB_NAME": "dagster", + "CDS_DAGSTER_DB_USER": "dagster_user", + "CDS_SUPERSET_DB_NAME": "superset", + "CDS_SUPERSET_DB_USER": "superset_user", + }.items() + ) + "\n" + + def test_team_prod_profile_reference_false_positives_are_gone(self): + profile_path = ( + _REPO_ROOT / "profiles" / "team-dagster-postgres-superset" / "profile.yaml" + ) + with tempfile.TemporaryDirectory() as tmp_dir: + env_file = Path(tmp_dir) / ".env" + env_file.write_text(self._ENV, encoding="utf-8") + findings, _diags = run_security_validation( + profile_path, + _RULE_SCHEMA_PATH, + _RULE_SET_PATH, + env_file=str(env_file), + environment="prod", + ) + + false_positives = [ + f for f in findings + if f["rule_id"] in {"CDS-SEC-001", "CDS-SEC-003", "CDS-SEC-013", "CDS-SEC-033"} + and ( + "initDbEnv" in f["path"] + or "waivers" in f["path"] + or f["path"].endswith("keyFrom") + or f["path"].endswith("encryptionKeyFrom") + ) + ] + self.assertEqual(false_positives, []) + class DeferredNoneScopeRuleDocumentationTest(unittest.TestCase): def test_each_remaining_none_scope_rule_declares_why_it_is_deferred(self):