Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions openspec/specs/roadmap-stage/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions src/devops_cli/ai/tasks/roadmap_stage_change.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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.
5 changes: 5 additions & 0 deletions src/devops_cli/lang/en/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
79 changes: 75 additions & 4 deletions src/devops_cli/roadmap/stage/stage_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: `#<number>: <reason>`
# (#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<number>[0-9]+):")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1032,22 +1063,59 @@ 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
quotes no line of `probes` when there are some, a task text that names a `MESSAGES.roadmap`
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))
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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`."
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
]
}
Loading
Loading