Files
werkrevisor/docs/prs/2026-10-05-PR#15-hard-orange-paths.md
claude-codeandClaude Opus 5.5 3e6c6b5424
ai-review/assessment AI findings; a developer approval after checking them unlocks the merge; a human review request is pending
ai-review/gate build succeeded after 00:14
werkator/server build succeeded after 00:00
werkator build succeeded after 00:13
werkator/werkrevisor-statistics build succeeded after 00:00
PR#15 doc lists the hsadmin-ng tiers as configured; the guide's tier bullets are clear; a test no longer sits after unittest.main()
Fixes the three findings of the self-review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 20:55:33 +02:00

192 lines
8.4 KiB
Markdown

# PR#15: critical paths give ORANGE
> **WARNING:** This document describes only the change applied in this PR.
> It may already be outdated once the next PR is merged.
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
## The Problem
werkrevisor 0.14.0 knows only one tier of risky paths: `[hard_yellow] paths` give YELLOW.
A change of the RBAC code, of the authentication or of a trust boundary gets the same verdict
as a changed build file or a new dependency.
In the approval mode, the reviewer cannot see in the verdict which PR needs a deliberate review.
## Non-Goals
- No review requests to dedicated reviewers per critical path; all ORANGE requests go to `human_reviewers`.
- No change of `.werkrevisor.toml` of werkrevisor itself;
the installed werkrevisor 0.14.0 does not know `[hard_orange]` and would give YELLOW for this PR.
- No new gate behavior: ORANGE from critical paths is unlocked as any other ORANGE.
## The Scenarios
### Feature: a changed critical path gives ORANGE
#### Background
- The config sets `[hard_orange] paths`, e.g. `**/auth/**`, `**/rbac/**` and `**/*secret*`.
- The config sets `[hard_yellow] paths`, e.g. `**/db/**` and `build.gradle*`.
#### Scenario#15.01: a changed critical path gives ORANGE with one reason line
So that the reviewer sees at once that the PR needs a deliberate review, however many critical files change.
- **Given** a PR changes 5 files below `src/auth/`
- **When** werkrevisor reviews the PR
- **Then** the verdict is ORANGE
- **and** one reason names 3 of the files with their glob and the count of the rest, e.g.
`critical paths changed, review them deliberately: src/auth/A0.java ([hard_orange] **/auth/**), … and 2 more`
##### Verified by
- [test_rules.HardOrangeTest.test_critical_path_gives_orange_with_one_aggregated_reason](../../tests/test_rules.py)
#### Scenario#15.02: documentation never matches a critical path
So that a PR document named after a critical topic does not need a deliberate review.
- **Given** a PR changes only `<file>`
- **When** werkrevisor reviews the PR
- **Then** no reason names critical paths
| file |
|---|
| `doc/PR/2026-08-17-PR#288-rbac-generator-hardening.md` |
| `docs/auth/README.md` |
##### Verified by
- [test_rules.HardOrangeTest.test_documentation_never_matches_critical_paths](../../tests/test_rules.py)
#### Scenario#15.03: a renamed file counts with its old path
So that a PR cannot move a critical file out of a critical path without a deliberate review.
- **Given** a PR renames `src/main/java/net/example/auth/Login.java` to `src/main/java/net/example/Login.java`
- **When** werkrevisor reviews the PR
- **Then** the verdict is ORANGE with the reason for critical paths
##### Verified by
- [test_rules.HardOrangeTest.test_renamed_from_critical_path_counts](../../tests/test_rules.py)
#### Scenario#15.04: a mechanical change of a critical path stays ORANGE
So that a critical path stays critical, also when the model need not review the change.
- **Given** a PR changes only the imports of `src/main/java/net/example/auth/Login.java`
- **When** werkrevisor reviews the PR
- **Then** the verdict is ORANGE with the reason for critical paths
- **and** the model gets no call
##### Verified by
- [test_mechanical.FlowTest.test_mechanical_change_of_a_critical_path_stays_orange](../../tests/test_mechanical.py)
#### Scenario#15.05: a file in both tiers is listed only as critical
So that the reasons name each file only once, under the more severe tier.
- **Given** the config lists `src/main/java/net/example/auth/**` in `[hard_orange] paths` and in `[hard_yellow] paths`
- **and** a PR changes `src/main/java/net/example/auth/Login.java`
- **When** werkrevisor reviews the PR
- **Then** the reason for critical paths names the file
- **and** no reason for risky paths names it
##### Verified by
- [test_rules.HardOrangeTest.test_file_in_both_tiers_is_listed_only_as_critical](../../tests/test_rules.py)
- [test_rules.HardOrangeTest.test_a_critical_db_schema_path_is_listed_only_as_critical](../../tests/test_rules.py)
#### Scenario#15.06: the repository config adds critical paths, but cannot remove them
So that a repository can declare its own critical paths, but cannot weaken the host config.
- **Given** the host config sets `[hard_orange] paths = ["**/auth/**"]`
- **and** `.werkrevisor.toml` sets `[hard_orange] paths = <repository paths>`
- **When** werkrevisor builds the config
- **Then** `[hard_orange] paths` are `<merged paths>`
| repository paths | merged paths |
|---|---|
| `["werkrevisor/gitea.py"]` | `["**/auth/**", "werkrevisor/gitea.py"]` |
| `[]` | `["**/auth/**"]` |
##### Verified by
- [test_config_layers.LayeringTest.test_hard_orange_paths_are_added](../../tests/test_config_layers.py)
- [test_config_layers.LayeringTest.test_repository_cannot_remove_hard_orange_paths](../../tests/test_config_layers.py)
#### Scenario#15.07: every config needs the section `[hard_orange]`
So that an operator who forgot the critical paths notices it at the first run.
- **Given** neither the host config nor `.werkrevisor.toml` sets `[hard_orange]`
- **When** werkrevisor loads the config
- **Then** werkrevisor stops with a config error that names `[hard_orange]`
##### Verified by
- [test_config_source_and_isolation.MissingSectionTest.test_a_config_without_hard_orange_is_refused](../../tests/test_config_source_and_isolation.py)
#### Scenario#15.08: the comment lists the critical paths before the YELLOW reasons
So that the reviewer reads first why the PR needs a deliberate review.
- **Given** a PR changes `src/main/java/net/example/auth/Login.java` and `build.gradle.kts`
- **When** werkrevisor writes the comment
- **Then** the reason for critical paths comes before the reason for risky paths
##### Verified by
- [test_flow.ApprovalGateTest.test_critical_paths_reason_comes_before_the_yellow_reasons](../../tests/test_flow.py)
#### Scenario#15.09: in the approval mode, a critical path waits for a human approval
So that a critical path never merges with the approval of the bot alone.
- **Given** the config sets `[gate] yellow_gate = "approval"` and `human_reviewers = ["alice"]`
- **and** a PR changes `src/main/java/net/example/auth/Login.java`
- **and** the model reports no finding
- **When** werkrevisor reviews the PR
- **Then** the bot submits no review
- **and** werkrevisor requests a review from alice
- **and** the status `ai-review/assessment` is `warning`
- **and** the exit code is 0
##### Verified by
- [test_flow.ApprovalGateTest.test_critical_path_requests_human_review_without_bot_review](../../tests/test_flow.py)
## The Solution
- The new section `[hard_orange]` has one key `paths`, a list of globs, as `[hard_yellow] paths`.
- The rules check each changed file, with its old path for a rename, against `[hard_orange] paths` first.
A match gives ORANGE with the reason code `critical_paths`;
the file is then not checked against the YELLOW paths, also not against the built-in ones.
- Files of the `docs` category never match, and mechanical files count as all other files.
- The repository layer adds its paths to the host paths, as for `[hard_yellow] paths`.
- `[hard_orange]` is a required section; an empty list is allowed.
- `config/hsadmin-ng.toml` moves RBAC, security, permissions and secrets from `[hard_yellow]` to `[hard_orange]`:
`**/rbac/**`, `src/main/resources/db/changelog/1-rbac/**`, `**/*Security*`, `**/*ApiKey*`, `**/*Auth*`, `**/*auth*`,
`**/*Jwt*`, `**/*Oidc*`, `**/*Rbac*`, `**/*rbac*`, `**/*Permission*`, `**/*secret*`, `**/*.pem`, `**/*.key`,
`.env*` and `.tc-environment`.
Build, CI, deployment, profiles and the DB schema stay in `[hard_yellow]`.
- `config/werkprobe.toml` moves authorization, RBAC and secrets to `[hard_orange]`.
- `config/werkrevisor.toml` sets `[hard_orange] paths = []`; the repository part follows in a separate PR.
- The README and the German operator docs describe `[hard_orange]`;
the proposal for hsadmin-ng lists the ORANGE and the YELLOW paths separately.
- werkrevisor 0.15.0.
Operators add `[hard_orange]` to their host configs before they install it.
## Additional Changes
- Three tests used an RBAC or authorization path of `config/werkprobe.toml` as a YELLOW path;
they now use a path that stays in `[hard_yellow]`.
## Follow-up PRs
- After the deploy of 0.15.0, `.werkrevisor.toml` moves the trust-boundary modules of werkrevisor
from `[hard_yellow]` to `[hard_orange]`.
- Review requests to dedicated reviewers per critical path.