Skip to content

Design question: with security-review-gate disabled, is the two-tier availability ruling still the right shape? #448

Closed

Description

@kyle-sexton

Summary

For the operator — a design question to rule on, not a defect to fix. No
change to .github/workflows/claude-security-review.yml is proposed here, and
none should be made until this is ruled on.

Org ruleset 19388547 (security-review-gate) is enforcement: disabled, so
security-review / security-review is a required status check on no
repository. The two-tier availability ruling in this lane's POSTURE exists to
manage the consequences of that check being REQUIRED. While the gate is
disabled, the harm the second tier is engineered to prevent cannot occur.

The question: is the two-tier ruling still the right shape, or should it be
revisited (or the gate re-armed)?

Evidence

Live ruleset read (gh api orgs/melodic-software/rulesets/19388547):

{"name":"security-review-gate","enforcement":"disabled","target":"branch",
 "conditions":{"repository_property":{"include":[{"name":"requires-security-review",
   "property_values":["true"]}]}},
 "rules":[{"type":"required_status_checks",
   "params":["security-review / security-review"]}]}

melodic-software/github-iac declares the same disabled state deliberately,
with rationale in OrgRulesets.cs: "Agentic review is advisory. GitHub Team
does not offer evaluation mode, so disabled is the only declarative state that
cannot make a model-backed action a merge dependency." (github-iac PR #282.)

Required checks actually in force on the only repo carrying
requires-security-review == "true"
(gh api repos/melodic-software/claude-code-plugins/rules/branches/main):

pr-title / pr-title
pr-issue-linkage / pr-issue-linkage
do-not-merge / do-not-merge
ci-status

security-review / security-review is absent.

What cuts each way

For leaving the ruling as-is — the POSTURE header is already conditional
where it matters most: "A consumer's ruleset may make the security-review
check REQUIRED". Read that way the machinery is deliberately future-proofed,
costs nothing while the gate is disabled, and is ready the moment it is
re-armed. Re-deriving it later is expensive and error-prone.

For revisiting it — other lines in the same file state the premise
unconditionally rather than conditionally, e.g. "the required check reports
green with NO execution evidence, so merges can and will land unreviewed during
an outage" and "where the check is the sole required security context". While
the gate is disabled those readings are stronger than reality: merges land
without execution evidence on EVERY run, not only during an outage, because
nothing gates on the context at all. A reader reasoning from those lines will
overestimate the control actually in force. The caller-drift tier's deliberate
red is likewise inert as a merge control.

Options (not recommendations)

  1. Leave the ruling unchanged; treat it as armed-and-waiting.
  2. Keep the behavior, but reword the unconditional lines to match the "may be
    required" framing already used in the POSTURE header.
  3. Re-arm the gate in github-iac (a policy change, operator-only), which
    would restore the premise the ruling assumes.
  4. Simplify the ruling to the single tier the disabled gate actually needs, and
    accept re-deriving the second tier if the gate is re-armed.

Related

The documentation half — the PLAN ledger asserting enforcement: active — is
corrected separately. This issue is deliberately left OPEN and unassigned; it
carries needs-human because an autonomous session must not resolve it.

No linked issue

Activity

  1. added
    status: needs-decisionAwaiting a human or maintainer judgment call.
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Aug 12, 2026
  2. removed
    status: needs-decisionAwaiting a human or maintainer judgment call.
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.needs-triageNot yet classified. Floor until a type and one priority tier are set.

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions