Skip to content

Commit ad58a25

Browse files
committed
fix(gator): surface PR-owned security regressions in reviews
Signed-off-by: John Myers <johntmyers@users.noreply.github.com>
1 parent 81f712e commit ad58a25

6 files changed

Lines changed: 42 additions & 3 deletions

File tree

‎.agents/skills/launch-openshell-gator/SKILL.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,11 @@ The launcher streams image-build and provisioning output to the terminal. Import
282282
- `/sandbox/.openshell-agent/history.jsonl` contains the latest 100 supervisor transitions, including active-cycle starts and completed cycle results.
283283
- `openshell-agent: still running watch cycle ...` is a heartbeat during long active model cycles.
284284
- `review_feedback_lookup_failed` means Gator could not build the required cross-SHA feedback ledger and deliberately skipped a context-free review.
285+
- Security regressions introduced or newly exposed by an unmerged PR belong
286+
in its normal review findings. Private follow-up under `SECURITY.md` is for
287+
pre-existing vulnerabilities independent of the PR; security classification
288+
alone is not a private-triage or test-dispatch gate. Gator must not initiate
289+
external security reporting without explicit operator authorization.
285290

286291
### Inspect Active Sandboxes
287292

‎.claude/agents/principal-engineer-reviewer.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,11 @@ When reviewing code or diffs:
9999
- Treat pre-existing security issues as private security follow-up, not public
100100
blockers on the current pull request. Treat other pre-existing defects as
101101
non-blocking follow-up work.
102+
- Surface security regressions introduced or newly exposed by an unmerged PR
103+
in the normal PR review evidence contract. A new reachable path, trusted sink,
104+
or security contract is PR-owned even when an underlying unsafe primitive
105+
already existed. Security classification alone is not a private-triage gate.
106+
Do not contact external reporting channels without operator authorization.
102107
- Keep docs, skill drift, diagnostic wording, and test-strength feedback
103108
advisory unless the published contract is materially false, the diagnostic
104109
creates an operational or safety failure, or missing coverage leaves a

‎scripts/agents/gator/agent.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# SPDX-License-Identifier: Apache-2.0
33

44
id: gator
5-
payload_version: 10
5+
payload_version: 12
66
display_name: Gator Gate Agent
77
description: Validate and monitor OpenShell GitHub issues and pull requests through the gator state machine.
88

‎scripts/agents/gator/bin/review_feedback_ledger_test.sh‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ rg -q 'COPY bin/validate-review-findings /usr/local/bin/validate-review-findings
349349
"$GATOR_DIR/Dockerfile"
350350
ruby -ryaml -e '
351351
manifest = YAML.load_file(ARGV.fetch(0))
352-
abort unless manifest.fetch("payload_version") == 9
352+
abort unless manifest.fetch("payload_version") == 12
353353
resource = manifest.fetch("resources").find {
354354
|entry| entry.fetch("id") == "gator-review-findings-schema"
355355
}
@@ -364,6 +364,14 @@ rg -q 'review-feedback-ledger NVIDIA OpenShell <pr-number>' \
364364
rg -q 'Every prior Gator finding is a durable review disposition' \
365365
"$GATOR_DIR/skills/gator-gate/SKILL.md"
366366
rg -q 'review feedback ledger' "$GATOR_DIR/prompts/gator.md"
367+
# Keep security routing consistent across the gate, top-level prompt, and
368+
# independent reviewer; PR-owned regressions must remain actionable reviews.
369+
rg -Fq 'PR in that PR' "$GATOR_DIR/skills/gator-gate/SKILL.md"
370+
rg -Fq 'private_security_review_required' "$GATOR_DIR/skills/gator-gate/SKILL.md"
371+
rg -Fq 'Do not initiate external disclosure' "$GATOR_DIR/skills/gator-gate/SKILL.md"
372+
rg -Fq 'not a generic private-security blocker' "$GATOR_DIR/prompts/gator.md"
373+
rg -Fq 'Security classification alone is not a private-triage gate' \
374+
"$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md"
367375
rg -q '### Pragmatic review calibration' \
368376
"$GATOR_DIR/skills/gator-gate/SKILL.md"
369377
rg -q 'A new commit permits a delta review' \

‎scripts/agents/gator/prompts/gator.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ Important sandbox constraints:
3434
- Use `gator:approval-needed` only when gator is complete but maintainer approval is still missing. Once maintainer approval is present and required checks remain green with no unresolved feedback, move to `gator:merge-ready` for the final merge or close decision.
3535
- Before running the `principal-engineer-reviewer` sub-agent or posting a review disposition, check existing gator comments and PR reviews for the current `headRefOid`. Do not run a reviewer or post another marked review/status disposition for a head SHA that already has one unless a maintainer explicitly requests a same-SHA public response, the PR is merged/closed and needs terminal cleanup, or the earlier attempt failed before posting. A prior marked comment that only says the reviewer sub-agent failed before producing output is a legacy infrastructure-failure report, not a valid review disposition; ignore it and retry the reviewer. A prior marked `## Blocked` comment whose only blocker was that the PR was draft is also not a valid code-review disposition after the PR becomes ready for review; ignore it for review suppression and run the reviewer once. Same-SHA CI changes, human replies, label changes, and reviewer comments must not create public status comments; record them only in the supervised result sentinel. A state-specific TTL nudge is the exception: after 48 business hours and no more often than once per 48 business hours for the same state and responsible actor, post the matching `## Author Follow-Up Nudge`, `## Maintainer Review Nudge`, `## Merge Decision Nudge`, or `## Blocker Follow-Up Nudge` template even when the head SHA is unchanged. A nudge must name the pending action, does not authorize a re-review, and does not consume or replace the one review disposition for that SHA.
3636
- When the gator skill requires the `principal-engineer-reviewer` sub-agent and the current effective patch has not already been reviewed by gator, first build the required review feedback ledger with `review-feedback-ledger`, then run a bounded independent review with `{{REVIEWER_COMMAND}}`. Treat the ledger's review mode, tree identity, patch identity, previous reviewed SHA, review budget, and telemetry as authoritative. Use the full PR diff for an initial review; for a follow-up, inspect unresolved feedback plus the author-only delta and do not mine unchanged or upstream-only code for new findings. Carry open findings without duplicating them, and preserve resolved or waived dispositions unless the new diff materially invalidates them.
37+
- Surface security regressions introduced or newly exposed by an unmerged PR
38+
as normal PR review findings, not a generic private-security blocker. Reserve
39+
private follow-up under `SECURITY.md` for pre-existing vulnerabilities
40+
independent of the PR. Do not initiate external reporting without explicit
41+
operator authorization; security classification alone does not suspend
42+
operator-authorized test dispatch or waive normal review blockers.
3743
- Require reviewer output to follow the JSON evidence contract in
3844
`/etc/openshell/agent-payload/skills/gator-gate/references/review-findings-schema.md`.
3945
Normalize it with `validate-review-findings`; only entries with

‎scripts/agents/gator/skills/gator-gate/SKILL.md‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,14 @@ If the `principal-engineer-reviewer` sub-agent fails before producing usable rev
4040
- You may push changes only when explicitly instructed by a GitHub comment from a maintainer or by a direct operator prompt.
4141
- Do not post `/ok to test <full-head-sha>` unless the current GitHub user has maintainer authority.
4242
- Code review is code-only. Do not run pre-commit, unit tests, or E2E locally as part of the initial PR review unless explicitly instructed.
43-
- Security vulnerabilities must not be triaged through public GitHub issues. Follow `SECURITY.md`.
43+
- Surface security-related findings introduced or newly exposed by an unmerged
44+
PR in that PR's normal review, using the same evidence, severity, and blocker
45+
rules as other findings. A security classification alone does not require
46+
private triage or suspend operator-authorized test dispatch.
47+
- For pre-existing vulnerabilities independent of the PR, follow `SECURITY.md`:
48+
retain detailed evidence privately and notify the operator without publishing
49+
exploit details. Do not initiate external disclosure, contact PSIRT, or submit
50+
a security report without explicit operator authorization.
4451

4552
Maintainer authority means one of:
4653

@@ -644,6 +651,14 @@ Keep reviews proportional, scope-bound, and convergent:
644651
- Route pre-existing security defects through the private security process.
645652
Do not publish exploit details or make them blockers on the current PR.
646653
Route other pre-existing defects to a non-blocking follow-up.
654+
- A pre-existing unsafe primitive does not make a new PR-owned exposure
655+
pre-existing. Compare base and head: if the PR creates a new reachable path,
656+
trusted sink, or security contract that makes the defect exploitable, surface
657+
the regression in the PR review with its invariant, impact, fix, and verification.
658+
Do not substitute a generic private-security notice for actionable feedback
659+
or invent a `private_security_review_required` gate solely because the finding
660+
is security-related. Existing review blockers still follow normal state rules;
661+
this distinction does not waive findings or authorize tests otherwise forbidden.
647662
- Treat docs, skill drift, diagnostic wording, and test-strength feedback as
648663
non-blocking unless the published contract is materially false, the
649664
diagnostic causes an operational or safety failure, or missing coverage

0 commit comments

Comments
 (0)