From d36b0fc86a172c7edf14f59872ade349c4045c76 Mon Sep 17 00:00:00 2001 From: Daniel Petty Date: Sat, 7 Nov 2026 22:02:12 +0000 Subject: [PATCH] feat(roadmap): require a security section for sensitive plans (#3247) Closes #3247 The proposal check now refuses a plan that touches a security trigger and gives no security text. The security triggers are SECURITY.md, pyproject.toml, paths under k8s/, and words such as credential or egress. The plan must answer what crosses a trust boundary, what a person must confirm, and what test fails if the rule breaks. The stage job renders this text as the ## Security section in design.md. --- .../.openspec.yaml | 0 .../design.md | 0 .../proposal.md | 0 .../specs/roadmap-stage/spec.md | 0 .../tasks.md | 14 +-- openspec/specs/roadmap-stage/spec.md | 48 +++++++++ .../ai/tasks/roadmap_stage_change.md | 2 + src/devops_cli/lang/en/messages.py | 5 + src/devops_cli/roadmap/stage/stage_change.py | 79 ++++++++++++++- .../case.json | 36 +++++++ .../facts.json | 10 ++ .../issue.txt | 9 ++ .../plan.json | 76 ++++++++++++++ .../plan.json | 3 +- .../stage/test_roadmap_stage_eval_cases.py | 8 +- .../stage/test_roadmap_stage_eval_command.py | 6 +- .../stage/test_roadmap_stage_eval_gates.py | 2 +- .../stage/test_roadmap_stage_security.py | 98 +++++++++++++++++++ 18 files changed, 376 insertions(+), 20 deletions(-) rename openspec/changes/{3247-require-a-security-section-for-sensitive => archive/2026-11-07-3247-require-a-security-section-for-sensitive}/.openspec.yaml (100%) rename openspec/changes/{3247-require-a-security-section-for-sensitive => archive/2026-11-07-3247-require-a-security-section-for-sensitive}/design.md (100%) rename openspec/changes/{3247-require-a-security-section-for-sensitive => archive/2026-11-07-3247-require-a-security-section-for-sensitive}/proposal.md (100%) rename openspec/changes/{3247-require-a-security-section-for-sensitive => archive/2026-11-07-3247-require-a-security-section-for-sensitive}/specs/roadmap-stage/spec.md (100%) rename openspec/changes/{3247-require-a-security-section-for-sensitive => archive/2026-11-07-3247-require-a-security-section-for-sensitive}/tasks.md (89%) create mode 100644 tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/case.json create mode 100644 tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/facts.json create mode 100644 tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/issue.txt create mode 100644 tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/plan.json create mode 100644 tests/roadmap/stage/test_roadmap_stage_security.py diff --git a/openspec/changes/3247-require-a-security-section-for-sensitive/.openspec.yaml b/openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/.openspec.yaml similarity index 100% rename from openspec/changes/3247-require-a-security-section-for-sensitive/.openspec.yaml rename to openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/.openspec.yaml diff --git a/openspec/changes/3247-require-a-security-section-for-sensitive/design.md b/openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/design.md similarity index 100% rename from openspec/changes/3247-require-a-security-section-for-sensitive/design.md rename to openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/design.md diff --git a/openspec/changes/3247-require-a-security-section-for-sensitive/proposal.md b/openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/proposal.md similarity index 100% rename from openspec/changes/3247-require-a-security-section-for-sensitive/proposal.md rename to openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/proposal.md diff --git a/openspec/changes/3247-require-a-security-section-for-sensitive/specs/roadmap-stage/spec.md b/openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/specs/roadmap-stage/spec.md similarity index 100% rename from openspec/changes/3247-require-a-security-section-for-sensitive/specs/roadmap-stage/spec.md rename to openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/specs/roadmap-stage/spec.md diff --git a/openspec/changes/3247-require-a-security-section-for-sensitive/tasks.md b/openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/tasks.md similarity index 89% rename from openspec/changes/3247-require-a-security-section-for-sensitive/tasks.md rename to openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/tasks.md index c3be782ee..425a66117 100644 --- a/openspec/changes/3247-require-a-security-section-for-sensitive/tasks.md +++ b/openspec/changes/archive/2026-11-07-3247-require-a-security-section-for-sensitive/tasks.md @@ -4,34 +4,34 @@ Reads: `src/devops_cli/roadmap/stage/stage_change.py`, `src/devops_cli/lang/en/messages.py`, `tests/roadmap/stage/test_roadmap_stage_plan.py`, `tests/support/builders.py`. Changes: `src/devops_cli/roadmap/stage/stage_change.py`, `src/devops_cli/lang/en/messages.py`, `tests/roadmap/stage/test_roadmap_stage_security.py` (new). -- [ ] 1.1 Write `test_a_plan_that_changes_security_md_with_no_security_text_is_refused` in `tests/roadmap/stage/test_roadmap_stage_security.py`: a test builds the plan's own `StageProposal` with its security text and asserts that `content_problems` returns no problem for it, and a second test builds the same proposal with an empty `security` and asserts that the problem names `SECURITY.md` +- [x] 1.1 Write `test_a_plan_that_changes_security_md_with_no_security_text_is_refused` in `tests/roadmap/stage/test_roadmap_stage_security.py`: a test builds the plan's own `StageProposal` with its security text and asserts that `content_problems` returns no problem for it, and a second test builds the same proposal with an empty `security` and asserts that the problem names `SECURITY.md` - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_security.py` fails on the base -- [ ] 1.2 Add `SECURITY_TRIGGER_PATHS` as a documented tuple subclass and `SECURITY_TRIGGER_WORDS` to `src/devops_cli/roadmap/stage/stage_change.py`, add `security` to `StageProposal`, add the trigger match and the refusal to `content_problems`, and add the message `stage_problem_security` to `src/devops_cli/lang/en/messages.py`. The test also asserts that every trigger path exists and that a plan with no trigger gets no problem +- [x] 1.2 Add `SECURITY_TRIGGER_PATHS` as a documented tuple subclass and `SECURITY_TRIGGER_WORDS` to `src/devops_cli/roadmap/stage/stage_change.py`, add `security` to `StageProposal`, add the trigger match and the refusal to `content_problems`, and add the message `stage_problem_security` to `src/devops_cli/lang/en/messages.py`. The test also asserts that every trigger path exists and that a plan with no trigger gets no problem - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_security.py` passes ## 2. Render the security section in the design file Reads: `src/devops_cli/roadmap/stage/stage_change.py`, `tests/roadmap/stage/test_roadmap_stage_plan.py`. Changes: `src/devops_cli/roadmap/stage/stage_change.py`, `tests/roadmap/stage/test_roadmap_stage_security.py`. -- [ ] 2.1 Write `test_the_design_file_holds_the_security_section` in `tests/roadmap/stage/test_roadmap_stage_security.py`: `render_change` of a proposal with security text writes a `## Security` section in `design.md`, and a proposal with an empty field writes none +- [x] 2.1 Write `test_the_design_file_holds_the_security_section` in `tests/roadmap/stage/test_roadmap_stage_security.py`: `render_change` of a proposal with security text writes a `## Security` section in `design.md`, and a proposal with an empty field writes none - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_security.py` fails on the base -- [ ] 2.2 Render the non-blank `security` text as a `## Security` section in `_design_file` of `src/devops_cli/roadmap/stage/stage_change.py`, after the rejected alternatives +- [x] 2.2 Render the non-blank `security` text as a `## Security` section in `_design_file` of `src/devops_cli/roadmap/stage/stage_change.py`, after the rejected alternatives - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_security.py` passes ## 3. State the security rule in the stage prompt Reads: `src/devops_cli/ai/tasks/roadmap_stage_change.md`, `tests/roadmap/stage/test_roadmap_stage_prompt_rules.py`. Changes: `src/devops_cli/ai/tasks/roadmap_stage_change.md`. -- [ ] 3.1 Add the `security` key to the reply keys and a rule that names the trigger paths and words, the three questions and the `## Security` section of `design.md` +- [x] 3.1 Add the `security` key to the reply keys and a rule that names the trigger paths and words, the three questions and the `## Security` section of `design.md` - Verification: `$ uv run devops ci test tests/roadmap/stage/test_roadmap_stage_prompt_rules.py` passes -- [ ] 3.2 Edit `src/devops_cli/ai/tasks/roadmap_stage_change.md` to state the security rule +- [x] 3.2 Edit `src/devops_cli/ai/tasks/roadmap_stage_change.md` to state the security rule - Verification: `$ uv run devops ci docs` passes ## 4. Measure the stage job with the new rule Reads: `src/devops_cli/roadmap/stage/stage_eval_cases.py`, `src/devops_cli/roadmap/stage/stage_change.py`. Changes: `src/devops_cli/roadmap/stage/stage_eval_cases.py`. -- [ ] 4.1 Add a golden case whose plan changes `SECURITY.md` with no security text and expects the security refusal, and run `uv run devops roadmap eval stage --replay` to record the score before and after the change. Fix any accepted case that the new rule now refuses by adding its security text +- [x] 4.1 Add a golden case whose plan changes `SECURITY.md` with no security text and expects the security refusal, and run `uv run devops roadmap eval stage --replay` to record the score before and after the change. Fix any accepted case that the new rule now refuses by adding its security text - Verification: `$ uv run devops roadmap eval stage --replay` passes ## Criteria diff --git a/openspec/specs/roadmap-stage/spec.md b/openspec/specs/roadmap-stage/spec.md index ccd8fafb6..a37393b2a 100644 --- a/openspec/specs/roadmap-stage/spec.md +++ b/openspec/specs/roadmap-stage/spec.md @@ -2823,3 +2823,51 @@ A stage round SHALL write the removals of closed items and the rejections of ite #### Scenario: Plan only - **WHEN** the command runs with `--plan` on a roadmap that holds a removal, a rejection and a ready item - **THEN** the store receives no write, no branch is pushed or deleted, and no GitHub write request is sent + +### Requirement: A security trigger needs a security section +The proposal check SHALL refuse a plan that names a security trigger and holds no non-blank `security` text, and SHALL name the matched path or word in the refusal. + +#### Scenario: A plan changes SECURITY.md with no security text +- **WHEN** `check_proposal` receives a proposal whose group changes `SECURITY.md` and whose `security` field is empty +- **THEN** it returns a problem that names `SECURITY.md` and asks for a non-blank `## Security` section + +#### Scenario: A task names a trigger word with no security text +- **WHEN** `check_proposal` receives a proposal whose task text names `credential` and whose `security` field is empty +- **THEN** it returns a problem that names `credential` + +#### Scenario: A plan with security text passes +- **WHEN** the same proposal holds non-blank `security` text +- **THEN** it returns no security problem + +#### Scenario: A plan with no trigger needs no security text +- **WHEN** `check_proposal` receives a proposal whose Changes, group paths and task texts name no trigger +- **THEN** it returns no security problem + +### Requirement: The security triggers are a fixed list of paths and words +The trigger paths MUST be the named constant `SECURITY_TRIGGER_PATHS` with a docstring, and every entry MUST be a path of the repository. + +#### Scenario: The trigger paths exist +- **WHEN** a test resolves each entry of `SECURITY_TRIGGER_PATHS` from the repository root +- **THEN** every entry is a file or a directory of the commit + +#### Scenario: A trigger path is missing +- **WHEN** an entry of `SECURITY_TRIGGER_PATHS` names no file and no directory +- **THEN** the test fails + +### Requirement: The design file holds the security section +`render_change` SHALL write a non-blank `security` field as a `## Security` section of `design.md`, and SHALL write no such section when the field is blank. + +#### Scenario: A plan with security text is rendered +- **WHEN** `render_change` renders a proposal whose `security` field holds non-blank text +- **THEN** `design.md` holds a `## Security` section with that text + +#### Scenario: A plan with no security text is rendered +- **WHEN** `render_change` renders a proposal whose `security` field is empty +- **THEN** `design.md` holds no `## Security` section + +### Requirement: The stage prompt names the security rule +The stage prompt MUST name the `security` key, the `## Security` section and the security trigger. + +#### Scenario: The prompt and the reply agree +- **WHEN** a test reads `roadmap_stage_change.md` and the fields of `StageProposal` +- **THEN** the prompt names `security` and the trigger, and the keys of the reply are the fields of the proposal diff --git a/src/devops_cli/ai/tasks/roadmap_stage_change.md b/src/devops_cli/ai/tasks/roadmap_stage_change.md index 2b54f2ee9..d9c23b0ea 100644 --- a/src/devops_cli/ai/tasks/roadmap_stage_change.md +++ b/src/devops_cli/ai/tasks/roadmap_stage_change.md @@ -30,6 +30,7 @@ The rules of the project: - A task group must not change or create a file under `openspec/specs/`, in `changes` or in `creates`. `openspec archive` writes the living specs from the spec delta of the change folder. - A group must not change or create `openspec/changes` or a path under it, because the job writes the change folder before the groups run. The check refuses a `changes` or `creates` path that is `openspec/changes` or sits under it. - The job checks every path against the files of the staged commit, and a path that is no file there is a fault. A path is a file of the commit, or a path that this group or an earlier group lists in `creates`. The check covers `reads`, `changes`, each path in a code span of a task, and each path in `verify`. Write every path of a task in a code span. Take a path from the `context` or from the issue only when the inputs show it exists: the issue can cite files of an older commit. When `problem` lists files with the same name, use one of them. A second reply with a missing path stops the item. +- When `what_changes`, the paths of a group or the text of a task name a security trigger, give a non-blank `security` text. The security trigger paths are `SECURITY.md`, `pyproject.toml` and paths under `k8s/`. The security trigger words are `sanitizer`, `credential`, `keyring`, `egress`, `admission policy`, `networkpolicy`, `rbac`, `new host`, `loosen`, `loosened` and `loosening`. The `security` text answers what crosses a trust boundary, what a person must confirm, and what test fails if the rule breaks. The job renders that text as the `## Security` section of `design.md`. When the plan names no security trigger, set `security` to an empty string. The reply has these keys and no other, at the top level or inside any entry: @@ -47,5 +48,6 @@ The reply has these keys and no other, at the top level or inside any entry: - `groups`: a list of objects with the keys named above. It has at least one. - `criteria`: a list of objects with the keys `criterion`, `group` and `reason`. The body of the issue may hold a list under a heading `Acceptance criteria`, written by a person before the block `devops-roadmap-refine`. Give one entry for each top-level item of that list, and none for the list in the refine block. `criterion` is the text of the item, copied character for character, without the `[ ]` mark. `group` is the number of the group that delivers it, counted from 1 in the order of `groups`, or the string `pending-person` when only a person can deliver it. `reason` is a string, and it is never blank when `group` is `pending-person`. An issue with no such list gets an empty list. The check refuses a reply that leaves one item out. - `dependencies`: a list of strings, one for each `overlaps` entry and each `metric_senders` entry that the context lists. Name the pull request number and the shared path, or the metric name and the other files. Give an empty list only when the context lists no overlap and no sender. +- `security`: a string that answers what crosses a trust boundary, what a person must confirm, and what test fails if the rule breaks when the plan names a security trigger, or an empty string. - Texts: no text of the plan is a placeholder such as `x`, `todo` or `none`, and the `why`, `existing_tool`, each line of `feasibility`, each decision and each task has at least 10 characters. The check refuses a reply that breaks this rule. The `summary` never ends with `...`. - Texts: the `why`, `existing_tool`, `purpose` and the `text` of a requirement are on one line and start with a word, never with `-`, `#`, `>`, `1.`, `[ ]`, a rule, a fence or an HTML tag. The check refuses a reply that breaks this rule. Any other text is joined to one line. diff --git a/src/devops_cli/lang/en/messages.py b/src/devops_cli/lang/en/messages.py index 539075d96..0ec1df055 100644 --- a/src/devops_cli/lang/en/messages.py +++ b/src/devops_cli/lang/en/messages.py @@ -3580,6 +3580,11 @@ class RoadmapMessages: "A group changes `{path}`, and does not list `{test}` in `reads` or `changes` or verify it. " "List `{test}` in `reads` or `changes`, and give a `verify` command that names `{test}`." ) + # A plan that names a security trigger has non-blank security text in design.md (#3247). + stage_problem_security: str = ( + "The plan matches the security trigger `{trigger}` and has no security text. Add a " + "non-blank `## Security` section to `design.md`." + ) # A plan states the work of each task once: a duplicate task spends the budget of the group on # no change (#3456). stage_group_one: str = "group {first}" diff --git a/src/devops_cli/roadmap/stage/stage_change.py b/src/devops_cli/roadmap/stage/stage_change.py index d905dc19e..bf910e0d9 100644 --- a/src/devops_cli/roadmap/stage/stage_change.py +++ b/src/devops_cli/roadmap/stage/stage_change.py @@ -168,6 +168,36 @@ PROMPT_SUFFIX: Final[str] = ".md" PROMPT_RULE_TEST: Final[str] = "tests/roadmap/stage/test_roadmap_stage_prompt_rules.py" _LEGACY_PROMPT_RULE_TEST: Final[str] = "tests/roadmap/test_roadmap_stage_prompt_rules.py" + + +class SecurityTriggerPaths(tuple[str, ...]): + """Paths that require a security section when changed (#3247): + - SECURITY.md: changes project security policies, disclosure, or requirements. + - pyproject.toml: adds or updates external dependencies. + - k8s/: modifies Kubernetes manifests, NetworkPolicies, or RBAC. + """ + + +SECURITY_TRIGGER_PATHS: Final[SecurityTriggerPaths] = SecurityTriggerPaths( + ( + "SECURITY.md", + "pyproject.toml", + "k8s/", + ) +) +SECURITY_TRIGGER_WORDS: Final[tuple[str, ...]] = ( + "sanitizer", + "credential", + "keyring", + "egress", + "admission policy", + "networkpolicy", + "rbac", + "new host", + "loosen", + "loosened", + "loosening", +) # An entry of `dependencies` that names an issue which must be delivered first: `#: ` # (#3224). An overlap or a metric sender keeps its own form, so the reader skips it. _ISSUE_DEPENDENCY: Final[re.Pattern[str]] = re.compile(r"^#(?P[0-9]+):") @@ -516,6 +546,7 @@ class StageProposal(_Reply): groups: list[StageGroup] = Field(min_length=1) criteria: list[StageCriterion] = Field(default_factory=list) dependencies: list[Line] = Field(default_factory=list) + security: Line = "" # The plan reader fills this list from `## Weakened checks` of `proposal.md` (#3490). The reply # schema does not show it: `render_change` writes no such section, so a key in a reply is lost. weakened_checks: SkipJsonSchema[list[WeakenedCheck]] = Field(default_factory=list) @@ -1032,6 +1063,41 @@ def eval_replay_problems(groups: Sequence[StageGroup]) -> list[str]: ] +def _security_search_texts(proposal: StageProposal) -> list[str]: + """The texts of `proposal` that a security trigger can match (#3247): the plan's Changes, + group paths and task texts.""" + texts: list[str] = list(proposal.what_changes) + for group in proposal.groups: + texts.extend(group.changes) + texts.extend(group.reads) + texts.extend(_group_task_texts(group)) + if group.process: + texts.append(group.process) + return texts + + +def _matched_security_trigger(proposal: StageProposal) -> str | None: + """The first security trigger in list order that `proposal` matches, or None.""" + texts = _security_search_texts(proposal) + for path in SECURITY_TRIGGER_PATHS: + path_lower = path.casefold() + if any(path_lower in t.casefold() for t in texts): + return path + for word in SECURITY_TRIGGER_WORDS: + word_lower = word.casefold() + if any(word_lower in t.casefold() for t in texts): + return word + return None + + +def _security_problems(proposal: StageProposal) -> list[str]: + """One problem when `proposal` matches a security trigger and has no security text (#3247).""" + trigger = _matched_security_trigger(proposal) + if trigger is not None and not proposal.security.strip(): + return [MESSAGES.roadmap.stage_problem_security.format(trigger=trigger)] + return [] + + def content_problems(proposal: StageProposal, probes: Sequence[str] = ()) -> list[str]: """What is wrong with the text of `proposal` that no schema says (#3200): a text that is too short or a placeholder word (`is_placeholder` of the pull request form), a `feasibility` that @@ -1039,15 +1105,17 @@ def content_problems(proposal: StageProposal, probes: Sequence[str] = ()) -> lis entry while no group changes the message file (#3405), two tasks that state the same work (#3456), a plan that builds its own code and gives no cost of tokens and time in `existing_tool`, a plan that adds a rule of the plan check and gives no test that accepts - its own plan (#3404), and a plan that changes a planner prompt and cites no prompt-rule test - (#3347). The replay (`stage_eval_gates`) and the job both call this through `check_proposal`, - so one rule set judges both.""" + its own plan (#3404), a plan that changes a planner prompt and cites no prompt-rule test + (#3347), and a plan that matches a security trigger with no security text (#3247). The replay + (`stage_eval_gates`) and the job both call this through `check_proposal`, so one rule set + judges both.""" texts = MESSAGES.roadmap problems = [p for field, text in _plan_texts(proposal) for p in _text_problems(field, text)] problems.extend(_message_file_problems(proposal)) problems.extend(_duplicate_task_problems(proposal)) problems.extend(_self_acceptance_problems(proposal)) problems.extend(_prompt_rule_problems(proposal)) + problems.extend(_security_problems(proposal)) lines = [_squash(line) for line in probes if line.strip()] if lines: feasibility = _squash(" ".join(proposal.feasibility)) @@ -1380,10 +1448,13 @@ def _design_file(proposal: StageProposal) -> str: rejected = _bullets( [f"**{r.alternative.strip()}**: {r.evidence.strip()}" for r in proposal.rejected] ) - return ( + content = ( f"# Design\n\n## Decisions\n\n{decisions or texts.stage_none}\n\n" f"## Rejected alternatives\n\n{rejected or texts.stage_none}\n" ) + if proposal.security.strip(): + content += f"\n## Security\n\n{proposal.security.strip()}\n" + return content def _paths(paths: Sequence[str], created: Collection[str] = ()) -> str: diff --git a/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/case.json b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/case.json new file mode 100644 index 000000000..a7e02737e --- /dev/null +++ b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/case.json @@ -0,0 +1,36 @@ +{ + "issue": 3247, + "pull": 3247, + "title": "feat(roadmap): require a security section in design.md for sensitive plans", + "change": "3247-require-a-security-section-for-sensitive", + "merge_commit": "0000000000000000000000000000000000000000", + "base_commit": "0000000000000000000000000000000000000000", + "verdict": "needs-fix", + "scores": { + "premise_ok": 5, + "fidelity": 4, + "task_quality": 4, + "rule_compliance": 3, + "accuracy": 3 + }, + "state": "open", + "summary": "The plan changes `SECURITY.md` and holds no security section. The stage check refuses the plan and requires a non-blank `## Security` section in `design.md`.", + "notes": [ + "The plan is one for #3247 that changes `SECURITY.md` with no security text, expecting the security refusal.", + "The change was never merged as this plan, so `pull`, `merge_commit` and `base_commit` hold placeholders: the case reads no commit, and the facts hold the files of its tree." + ], + "defects": [ + { + "type": "other", + "severity": "major", + "where": "design.md", + "problem": "The plan changes `SECURITY.md` and holds no security section.", + "evidence": "Group 1 changes `SECURITY.md` and `security` is empty.", + "gate": "content", + "paths": [ + "SECURITY.md" + ], + "message": "Add a non-blank `## Security` section to `design.md`." + } + ] +} diff --git a/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/facts.json b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/facts.json new file mode 100644 index 000000000..4f9e135c7 --- /dev/null +++ b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/facts.json @@ -0,0 +1,10 @@ +{ + "tracked": [ + "SECURITY.md", + "src/devops_cli/roadmap/stage/stage_change.py", + "tests/roadmap/stage/test_roadmap_stage_security.py" + ], + "capabilities": [ + "roadmap-stage" + ] +} diff --git a/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/issue.txt b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/issue.txt new file mode 100644 index 000000000..4c1b26d3e --- /dev/null +++ b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/issue.txt @@ -0,0 +1,9 @@ +# Item #3247: feat(roadmap): require a security section in design.md for sensitive plans + +## Problem or Motivation + +Security and scope questions are left to the model, so plan reviews and fixes returned findings that needed a person, such as a loosened guard, a stored URL, ambient environment variables and a credential redaction in the wrong layer. A deterministic trigger and a non-blank security section make the plan answer what crosses a trust boundary, what a person must confirm, and what test fails if the rule breaks. + +### Acceptance criteria + +- [ ] The proposal check refuses a plan that touches a security trigger and holds no security text. diff --git a/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/plan.json b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/plan.json new file mode 100644 index 000000000..1b0eaa0e2 --- /dev/null +++ b/tests/fixtures/roadmap_eval/stage/3247-a-plan-that-changes-security-md/plan.json @@ -0,0 +1,76 @@ +{ + "summary": "require a security section for sensitive plans", + "pull_request_body": "This change adds a security section requirement for plans that touch security triggers.", + "why": "Issue #3247: security and scope questions are left to the model, so plan reviews and fixes returned findings that needed a person. A deterministic trigger and a non-blank security section make the plan answer what crosses a trust boundary, what a person must confirm, and what test fails if the rule breaks.", + "what_changes": [ + "Refuse a plan that changes `SECURITY.md` with no security text in `src/devops_cli/roadmap/stage/stage_change.py`." + ], + "skip_specs": false, + "feasibility": [ + "`check_proposal` of `src/devops_cli/roadmap/stage/stage_change.py` checks content problems." + ], + "existing_tool": "The change adds about 90 lines to `src/devops_cli/roadmap/stage/stage_change.py` at a cost of 30k tokens and 4 hours to build, keep and fix.", + "builds_own": true, + "decisions": [ + { + "decision": "Put refusal in content_problems", + "reason": "It collects plan text rules." + } + ], + "rejected": [ + { + "alternative": "Let model edit design.md directly", + "evidence": "render_change writes design.md." + } + ], + "specs": [ + { + "capability": "roadmap-stage", + "purpose": null, + "added": [ + { + "name": "Require security section for sensitive plans", + "text": "The proposal check SHALL refuse a plan that names a security trigger and holds no security text.", + "scenarios": [ + { + "name": "Plan changes SECURITY.md", + "when": "A plan changes SECURITY.md with no security text", + "then": "The check returns a security problem" + } + ] + } + ], + "modified": [] + } + ], + "groups": [ + { + "title": "Refuse a plan that changes SECURITY.md with no security text", + "kind": "behavior", + "reads": [ + "SECURITY.md" + ], + "changes": [ + "SECURITY.md", + "src/devops_cli/roadmap/stage/stage_change.py", + "tests/roadmap/stage/test_roadmap_stage_security.py" + ], + "creates": [], + "creates_lines": {}, + "test": "test_a_plan_that_changes_security_md_with_no_security_text_is_refused", + "implementation": "Check for security trigger and require non-blank security text", + "documentation": null, + "verify": "uv run devops ci test tests/roadmap/stage/test_roadmap_stage_security.py", + "process": null + } + ], + "criteria": [ + { + "criterion": "The proposal check refuses a plan that touches a security trigger and holds no security text.", + "group": 1, + "reason": null + } + ], + "dependencies": [], + "security": "" +} diff --git a/tests/fixtures/roadmap_eval/stage/good-2739-the-subprocess-environment-guard-accepts-gh/plan.json b/tests/fixtures/roadmap_eval/stage/good-2739-the-subprocess-environment-guard-accepts-gh/plan.json index 6ea8c83c5..fa8c4a1f2 100644 --- a/tests/fixtures/roadmap_eval/stage/good-2739-the-subprocess-environment-guard-accepts-gh/plan.json +++ b/tests/fixtures/roadmap_eval/stage/good-2739-the-subprocess-environment-guard-accepts-gh/plan.json @@ -81,5 +81,6 @@ "process": null, "kind": "behavior" } - ] + ], + "security": "A caller cannot redirect telemetry egress across the trust boundary to an unapproved destination. A person confirms that only destination keys are refused. The test test_build_subprocess_env_refuses_gh_telemetry_endpoint_url fails if the rule breaks." } diff --git a/tests/roadmap/stage/test_roadmap_stage_eval_cases.py b/tests/roadmap/stage/test_roadmap_stage_eval_cases.py index 0a479ae38..c4b6e8bce 100644 --- a/tests/roadmap/stage/test_roadmap_stage_eval_cases.py +++ b/tests/roadmap/stage/test_roadmap_stage_eval_cases.py @@ -40,7 +40,7 @@ {2975, 2976, 2977, 2978, 2979, 2980, 2981, 2982, 2983, 2984, 2985, 2986, 2987, 2988, 2989} | {2990, 2991, 2992, 2993, 2994, 2995, 2996, 2997, 2998, 2999, 3001, 3002} | {2719, 2761, 2766, 2857, 2858, 2859, 2870, 2958, 2971, 2972} - | {3167, 3173, 3174, 3331, 3378, 3411, 3435} + | {3167, 3173, 3174, 3331, 3378, 3411, 3435, 3247} ) SHA_LENGTH = 40 FULL_HASH = re.compile(r"[0-9a-f]{40}") @@ -51,12 +51,12 @@ def cases() -> list[StageCase]: return load_cases(REPO_ROOT / CASES_DIRECTORY) -def test_the_set_holds_the_44_plans_and_four_good_ones(cases: list[StageCase]) -> None: +def test_the_set_holds_the_45_plans_and_four_good_ones(cases: list[StageCase]) -> None: reviewed = {case.pull for case in cases if case.verdict is not Verdict.GOOD} good = [case for case in cases if case.verdict is Verdict.GOOD] - assert (frozenset(reviewed), len(cases), len(good)) == (EVALUATED_PULLS, 48, 4) - assert len(EVALUATED_PULLS) == 44 + assert (frozenset(reviewed), len(cases), len(good)) == (EVALUATED_PULLS, 49, 4) + assert len(EVALUATED_PULLS) == 45 def test_a_good_case_is_the_twin_of_a_reviewed_case_and_has_no_defect( diff --git a/tests/roadmap/stage/test_roadmap_stage_eval_command.py b/tests/roadmap/stage/test_roadmap_stage_eval_command.py index 10ae7a506..9df6a96e1 100644 --- a/tests/roadmap/stage/test_roadmap_stage_eval_command.py +++ b/tests/roadmap/stage/test_roadmap_stage_eval_command.py @@ -75,10 +75,10 @@ def test_the_replay_exits_0_and_lists_what_each_gate_caught(cli: Cli) -> None: result = _eval(cli, "--replay") assert result.exit_code == 0, result.output - assert "replay through the gates: 48 case(s), 4 good" in result.output + assert "replay through the gates: 49 case(s), 4 good" in result.output assert "paths #3010" in result.output assert "spec_delta_wrong" in result.output - assert "Defects that a gate judges: 70 of 307 (23%)." in result.output + assert "Defects that a gate judges: 71 of 308 (23%)." in result.output assert "Defects that wait for a gate" not in result.output assert "Good plans that every gate accepts: 4 of 4." in result.output assert "Result: every gate refuses what it must refuse." in result.output @@ -96,7 +96,7 @@ def test_the_replay_as_json_holds_the_totals_of_each_gate(cli: Cli) -> None: ("content", 0), ] assert report["failures"] == [] - assert report["defects"] == {"judged": 70, "known": 307} + assert report["defects"] == {"judged": 71, "known": 308} assert {t["type"]: (t["caught"], t["missed"], t["unjudged"]) for t in report["types"]}[ "verify_form" ] == (6, 0, 12) diff --git a/tests/roadmap/stage/test_roadmap_stage_eval_gates.py b/tests/roadmap/stage/test_roadmap_stage_eval_gates.py index d99d86b1b..2ffdee947 100644 --- a/tests/roadmap/stage/test_roadmap_stage_eval_gates.py +++ b/tests/roadmap/stage/test_roadmap_stage_eval_gates.py @@ -107,7 +107,7 @@ def test_the_counts_of_each_type_are_the_baseline_that_a_change_of_a_gate_beats( assert counts[DefectType.VERIFY_FORM] == (6, 0, 12) assert counts[DefectType.TASK_ORDER] == (1, 0, 8) assert counts[DefectType.IGNORES_EXISTING_CODE] == (0, 0, 23) - assert report.judged() == (70, 307) + assert report.judged() == (71, 308) def test_the_judged_share_counts_the_caught_and_the_missed_and_not_the_rest( diff --git a/tests/roadmap/stage/test_roadmap_stage_security.py b/tests/roadmap/stage/test_roadmap_stage_security.py new file mode 100644 index 000000000..f2cf822f6 --- /dev/null +++ b/tests/roadmap/stage/test_roadmap_stage_security.py @@ -0,0 +1,98 @@ +"""`devops roadmap stage`: require a security section in design.md for sensitive plans (#3247).""" + +from __future__ import annotations + +from datetime import date +from pathlib import Path + +from devops_cli.ai.task_loader import load_task_prompt +from devops_cli.roadmap.stage.stage_change import ( + SECURITY_TRIGGER_PATHS, + content_problems, + render_change, +) +from tests.support.builders import make_stage_proposal + + +def test_a_plan_that_changes_security_md_with_no_security_text_is_refused() -> None: + sample = make_stage_proposal() + group = sample.groups[0].model_copy( + update={ + "changes": ["SECURITY.md", "src/devops_cli/roadmap/stage/stage_change.py"], + "creates": [], + "creates_lines": {}, + "test": ( + "Write a test that builds the plan's own `StageProposal` and asserts that " + "`content_problems` returns no problem for it" + ), + } + ) + refused = make_stage_proposal(groups=[group], security="") + problems = content_problems(refused) + assert any("SECURITY.md" in p and "## Security" in p for p in problems) + + accepted = make_stage_proposal( + groups=[group], + security=( + "Answers what crosses a trust boundary, what a person must confirm, and what test " + "fails if the rule breaks." + ), + ) + assert content_problems(accepted) == [] + + +def test_a_task_that_names_a_trigger_word_with_no_security_text_is_refused() -> None: + sample = make_stage_proposal() + group = sample.groups[0].model_copy( + update={"implementation": "Store the credential in the keyring"} + ) + refused = make_stage_proposal(groups=[group], security="") + problems = content_problems(refused) + assert any("credential" in p for p in problems) + + +def test_a_plan_with_no_trigger_needs_no_security_text() -> None: + proposal = make_stage_proposal(security="") + assert content_problems(proposal) == [] + + +def test_every_trigger_path_exists() -> None: + for path_str in SECURITY_TRIGGER_PATHS: + assert Path(path_str).exists(), f"Trigger path {path_str} does not exist" + + +def test_the_trigger_path_constant_is_documented() -> None: + assert SECURITY_TRIGGER_PATHS.__doc__ is not None + assert len(SECURITY_TRIGGER_PATHS.__doc__.strip()) > 0 + + +def test_the_design_file_holds_the_security_section() -> None: + security_text = "Answers trust boundary, human confirmation, and failure tests." + proposal_with_security = make_stage_proposal(security=security_text) + files_with = render_change( + "sample-change", + proposal_with_security, + ["comment-thread"], + created=date(2026, 11, 7), + ) + design_with = files_with["openspec/changes/sample-change/design.md"] + assert "## Security" in design_with + assert security_text in design_with + + proposal_no_security = make_stage_proposal(security="") + files_without = render_change( + "sample-change", + proposal_no_security, + ["comment-thread"], + created=date(2026, 11, 7), + ) + design_without = files_without["openspec/changes/sample-change/design.md"] + assert "## Security" not in design_without + + +def test_the_stage_prompt_names_the_security_rule() -> None: + prompt = load_task_prompt("roadmap_stage_change.md") + assert "`security`:" in prompt + assert "## Security" in prompt + assert "SECURITY.md" in prompt + assert "credential" in prompt