diff --git a/.agents/skills/launch-openshell-gator/SKILL.md b/.agents/skills/launch-openshell-gator/SKILL.md index 93addd1119..2a516754ee 100644 --- a/.agents/skills/launch-openshell-gator/SKILL.md +++ b/.agents/skills/launch-openshell-gator/SKILL.md @@ -286,6 +286,11 @@ The launcher streams image-build and provisioning output to the terminal. Import size-limited full-diff endpoint. An unavailable patch ID alone does not block the ledger; it disables rebase-equivalence shortcuts. Reviewers should inspect oversized changes file-by-file locally. +- Security regressions introduced or newly exposed by an unmerged PR belong + in its normal review findings. Private follow-up under `SECURITY.md` is for + pre-existing vulnerabilities independent of the PR; security classification + alone is not a private-triage or test-dispatch gate. Gator must not initiate + external security reporting without explicit operator authorization. ### Inspect Active Sandboxes diff --git a/.claude/agents/principal-engineer-reviewer.md b/.claude/agents/principal-engineer-reviewer.md index ee31612144..f944fbf7af 100644 --- a/.claude/agents/principal-engineer-reviewer.md +++ b/.claude/agents/principal-engineer-reviewer.md @@ -99,6 +99,11 @@ When reviewing code or diffs: - Treat pre-existing security issues as private security follow-up, not public blockers on the current pull request. Treat other pre-existing defects as non-blocking follow-up work. +- Surface security regressions introduced or newly exposed by an unmerged PR + in the normal PR review evidence contract. A new reachable path, trusted sink, + or security contract is PR-owned even when an underlying unsafe primitive + already existed. Security classification alone is not a private-triage gate. + Do not contact external reporting channels without operator authorization. - Keep docs, skill drift, diagnostic wording, and test-strength feedback advisory unless the published contract is materially false, the diagnostic creates an operational or safety failure, or missing coverage leaves a diff --git a/scripts/agents/gator/agent.yaml b/scripts/agents/gator/agent.yaml index 5aacfb136e..b1e7cc1e8a 100644 --- a/scripts/agents/gator/agent.yaml +++ b/scripts/agents/gator/agent.yaml @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 id: gator -payload_version: 11 +payload_version: 12 display_name: Gator Gate Agent description: Validate and monitor OpenShell GitHub issues and pull requests through the gator state machine. diff --git a/scripts/agents/gator/bin/review_feedback_ledger_test.sh b/scripts/agents/gator/bin/review_feedback_ledger_test.sh index e671ef4562..c97f5b941c 100755 --- a/scripts/agents/gator/bin/review_feedback_ledger_test.sh +++ b/scripts/agents/gator/bin/review_feedback_ledger_test.sh @@ -406,7 +406,7 @@ rg -q 'COPY bin/validate-review-findings /usr/local/bin/validate-review-findings "$GATOR_DIR/Dockerfile" ruby -ryaml -e ' manifest = YAML.load_file(ARGV.fetch(0)) - abort unless manifest.fetch("payload_version") == 11 + abort unless manifest.fetch("payload_version") == 12 resource = manifest.fetch("resources").find { |entry| entry.fetch("id") == "gator-review-findings-schema" } @@ -421,6 +421,14 @@ rg -q 'review-feedback-ledger NVIDIA OpenShell ' \ rg -q 'Every prior Gator finding is a durable review disposition' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'review feedback ledger' "$GATOR_DIR/prompts/gator.md" +# Keep security routing consistent across the gate, top-level prompt, and +# independent reviewer; PR-owned regressions must remain actionable reviews. +rg -Fq 'PR in that PR' "$GATOR_DIR/skills/gator-gate/SKILL.md" +rg -Fq 'private_security_review_required' "$GATOR_DIR/skills/gator-gate/SKILL.md" +rg -Fq 'Do not initiate external disclosure' "$GATOR_DIR/skills/gator-gate/SKILL.md" +rg -Fq 'not a generic private-security blocker' "$GATOR_DIR/prompts/gator.md" +rg -Fq 'Security classification alone is not a private-triage gate' \ + "$GATOR_DIR/../../../.claude/agents/principal-engineer-reviewer.md" rg -q '### Pragmatic review calibration' \ "$GATOR_DIR/skills/gator-gate/SKILL.md" rg -q 'A new commit permits a delta review' \ diff --git a/scripts/agents/gator/prompts/gator.md b/scripts/agents/gator/prompts/gator.md index 27cba2a0c8..d4c707b3db 100644 --- a/scripts/agents/gator/prompts/gator.md +++ b/scripts/agents/gator/prompts/gator.md @@ -34,6 +34,12 @@ Important sandbox constraints: - 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. - 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. - 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. +- Surface security regressions introduced or newly exposed by an unmerged PR + as normal PR review findings, not a generic private-security blocker. Reserve + private follow-up under `SECURITY.md` for pre-existing vulnerabilities + independent of the PR. Do not initiate external reporting without explicit + operator authorization; security classification alone does not suspend + operator-authorized test dispatch or waive normal review blockers. - Require reviewer output to follow the JSON evidence contract in `/etc/openshell/agent-payload/skills/gator-gate/references/review-findings-schema.md`. Normalize it with `validate-review-findings`; only entries with diff --git a/scripts/agents/gator/skills/gator-gate/SKILL.md b/scripts/agents/gator/skills/gator-gate/SKILL.md index 3509984bed..efef39291c 100644 --- a/scripts/agents/gator/skills/gator-gate/SKILL.md +++ b/scripts/agents/gator/skills/gator-gate/SKILL.md @@ -40,7 +40,14 @@ If the `principal-engineer-reviewer` sub-agent fails before producing usable rev - You may push changes only when explicitly instructed by a GitHub comment from a maintainer or by a direct operator prompt. - Do not post `/ok to test ` unless the current GitHub user has maintainer authority. - 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. -- Security vulnerabilities must not be triaged through public GitHub issues. Follow `SECURITY.md`. +- Surface security-related findings introduced or newly exposed by an unmerged + PR in that PR's normal review, using the same evidence, severity, and blocker + rules as other findings. A security classification alone does not require + private triage or suspend operator-authorized test dispatch. +- For pre-existing vulnerabilities independent of the PR, follow `SECURITY.md`: + retain detailed evidence privately and notify the operator without publishing + exploit details. Do not initiate external disclosure, contact PSIRT, or submit + a security report without explicit operator authorization. Maintainer authority means one of: @@ -651,6 +658,14 @@ Keep reviews proportional, scope-bound, and convergent: - Route pre-existing security defects through the private security process. Do not publish exploit details or make them blockers on the current PR. Route other pre-existing defects to a non-blocking follow-up. +- A pre-existing unsafe primitive does not make a new PR-owned exposure + pre-existing. Compare base and head: if the PR creates a new reachable path, + trusted sink, or security contract that makes the defect exploitable, surface + the regression in the PR review with its invariant, impact, fix, and verification. + Do not substitute a generic private-security notice for actionable feedback + or invent a `private_security_review_required` gate solely because the finding + is security-related. Existing review blockers still follow normal state rules; + this distinction does not waive findings or authorize tests otherwise forbidden. - Treat docs, skill drift, diagnostic wording, and test-strength feedback as non-blocking unless the published contract is materially false, the diagnostic causes an operational or safety failure, or missing coverage