Repository navigation
fix(security): stop flagging renderer references and waiver text as secrets (#979) - #1039
Merged
RonaldHensbergen merged 5 commits intoOct 31, 2026
Conversation
…ecrets (#979) `cds security team-dagster-postgres-superset --environment prod` reported 11 false positives whose values are not secrets: - CDS-SEC-001/033 on initDbEnv.*_DB_PASSWORD and CDS-SEC-003 on initDbEnv.*_DB_{USER,PASSWORD}: the values are renderer references (config.passwordFrom, config.dagsterDatabase.username, ...) that the declarative match engine sees unresolved in the planned config, while the renderer resolves them via _merge_init_db_env/_resolve_expr and the rendered Compose output contains only ${CDS_*} placeholders. - CDS-SEC-003 on security.waivers.runtimePolicy.reason: the documented entropy exemption pattern spec.security.waivers.*.reason never matched because _flatten_profile_by_module emits spec-level keys without the spec. prefix. - CDS-SEC-031 on the project-root .env when the CLI was invoked from outside the project (e.g. CDS_PROFILE_PATH set): the env path was normalized against cwd only, so an absolute root .env matched */*.env. - CDS-SEC-013 on backup.encryption.keyFrom/encryptionKeyFrom: the shared secrets.backup_encryption_key reference was treated as a reused value. Changes: - New opt-in rule flag ignoreRendererReferences (rule-schema.json, rule set 1.0.1) set on CDS-SEC-001/003/033: values that are entirely a renderer reference (bare config.x/secrets.x/bindings.x/service.x/ certificates.x or the ${...}-wrapped form, per renderer._resolve_expr) are skipped, same as ${secrets.*} was already. - _check_secret_reuse skips renderer references. - The waiver-reason entropy exemption also matches the unprefixed security.waivers.*.reason path. - CDS-SEC-031 now matches the env file path relative to the project root (resolve_project_root, moved from cli/main.py to cli/security_common.py so the scan shares one implementation), so a bare project-root .env never fires regardless of invocation directory. - Regression tests for all four cases plus an end-to-end run of the reference prod profile; docs expected-findings table and CHANGELOG updated. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
RonaldHensbergen
requested review from
SemTiOne and
vanderheijden86
as code owners
October 30, 2026 22:19
…loud findings - .github/workflows/ci.yml: the "Run end-to-end smoke test" step asserted `[FAIL] security` and exit 1 for `cds test local-dagster-postgres-superset` because CDS-SEC-001/003 used to fire on the initDbEnv renderer references. Since #979 those false positives are gone, so with CI's env-derived secret values the security stage passes (only the known MEDIUM CDS-SEC-080/081 gaps remain); the step now asserts `[PASS] security` and exit 0. Image verification is unchanged: the "Verify unverifiable images fail closed" step still expects `cds security --verify-images` to exit 1 (CDS-VER-*, cosign absent on the runner). - CHANGELOG.md: collapse a double blank line after the #979 entry (markdownlint MD012). - tests/test_security.py: factor the shared tmp-project/cwd-elsewhere setup of the two CDS-SEC-031 tests into one helper (SonarCloud new_duplicated_lines_density was 4.8 > 3.0 gate). - tests/test_cds_workflow.py: the init'd-project `cds test` still fails security, but now via CDS-SEC-012 on the generated placeholder .env values, not CDS-SEC-001/003 — docstring updated to match. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
…erer-reference-false-positives
…d k8s renderers (#979) SonarCloud flagged 4.8% duplication on new code in PR #1039 (gate: <=3%): a 28-line block building and validating provided-contract host candidates was identical in cli/renderer.py::_validate_provided_contract_hosts and cli/k8s_renderer.py::_check_provided_contract_services. Extract it into cli/renderer.py::_contract_host_candidates and call it from both. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
…erer-reference-false-positives # Conflicts: # cli/k8s_renderer.py # cli/renderer.py
|
RonaldHensbergen
deleted the
fix/979-security-renderer-reference-false-positives
branch
October 31, 2026 13:31
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
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.
Fixes #979.
cds security team-dagster-postgres-superset --environment prodreported 11 false positives whose values are not secrets. This PR removes all four root causes:initDbEnv.*_DB_PASSWORD, CDS-SEC-003 oninitDbEnv.*_DB_{USER,PASSWORD}— the values are renderer references (config.passwordFrom,config.dagsterDatabase.username, ...) that the declarative engine sees unresolved in the planned config; the renderer resolves them (_merge_init_db_env/_resolve_expr) and rendered output contains only${CDS_*}placeholders. New opt-in rule flagignoreRendererReferences(rule-schema.json + rule set 1.0.1) on CDS-SEC-001/003/033 skips values that are entirely a renderer reference (bare or${...}-wrappedconfig|secrets|bindings|service|certificates.*).security.waivers.runtimePolicy.reason— the entropy exemption patternspec.security.waivers.*.reasonnever matched because spec-level keys flatten without thespec.prefix; both forms are now exempt..envwhen invoked from outside the project (CDS_PROFILE_PATH) — the env path is now matched relative to the project root (resolve_project_root, moved tocli/security_common.pyand shared withcli/main.py), so a bare root.envnever matches*/*.envregardless of cwd.backup.encryption.keyFrom/encryptionKeyFrom— the sharedsecrets.backup_encryption_keyreference is no longer treated as a reused secret value.Verified end-to-end: from an unrelated cwd with
CDS_PROFILE_PATHset,cds security team-dagster-postgres-superset --environment prodnow reports only the documented true positives/known gaps (CDS-SEC-074 x2, 080, 081 x3, W103).Tests: 13 new regression tests in
tests/test_security.py(reference detection, per-rule opt-in gate, literals still flagged, reuse check, waiver path, CDS-SEC-031 from outside the project root, end-to-end reference prod profile). Full suite: 1954 passed, 45 skipped (1 pre-existing environmental SQLAlchemy failure, fails on base too). Ruff + markdownlint clean on touched files. CHANGELOG[Unreleased]and the expected-findings table indocs/production-hardening-checklist.mdupdated.Note: the working tree also contained unrelated uncommitted work (issue #1029/#1035); this PR contains only the #979 files.