Skip to content

fix(triage-security): bind action plans to trusted issue scope - #319

Merged
mrizzi merged 3 commits into
RHEcosystemAppEng:verify-pr-fullsendfrom
mrizzi:TC-6646
Sep 30, 2026
Merged

mrizzi merged 3 commits into
RHEcosystemAppEng:verify-pr-fullsendfrom
mrizzi:TC-6646

Conversation

@mrizzi

@mrizzi mrizzi commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

An authorization grant for one triage issue previously allowed a schema-valid plan to mutate unrelated Jira issues. The executor now preflights the entire plan against trusted issue identity, prefetched triage relationships, and remediation project context before any Jira read or write.

References must be established in order without rebinding. Retry markers cannot bypass authorization or trigger digest repair before validation. Related-target field and status actions use retry suppression only when the snapshot belongs to their target. Report-only behavior, generated references, digest ordering, and existing remediation reuse remain covered.

Validation:

  • Original TC-8100/TC-42 reproducer failed before the fix and passes afterward.
  • Focused executor and Fullsend contract tests: 64 passed.
  • Complete scripts suite: 259 passed.
  • Plugin manifest validation and git diff --check passed.
  • Skillsaw: 0 errors; 7 warnings in unchanged files.
  • Independent code review found no remaining blocking issues.

Implements TC-6646

Summary by Sourcery

Preflight triage security action plans against trusted issue scope and execution context before allowing Jira operations.

Bug Fixes:

  • Prevent mutation-authorized triage plans from targeting Jira issues outside the trusted issue and documented related scope.
  • Reject invalid reference rebinding, unauthorized retries, mismatched remediation projects, and unresolved references before any Jira reads or writes.
  • Preserve safe retry handling for already-applied related-target status transitions.

Enhancements:

  • Centralize action requirements and authorization target declarations for complete-plan preflight validation.
  • Expand trusted target resolution to include documented triage relationships, sibling searches, and existing remediation context.

Tests:

  • Add executor and Fullsend contract coverage for scope enforcement, retry suppression, reference binding, related targets, project validation, CLI failures, and no-call preflight behavior.

Preflight all targets, references, and remediation projects before Jira operations or retry suppression. Keep related targets independent of the primary issue retry snapshot.

Implements TC-6646

Assisted-by: Claude Code
@sourcery-ai

sourcery-ai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer's Guide

The executor now fails closed by preflighting every mutation, reference, retry marker, and remediation context against trusted triage scope before any Jira operation, while preserving valid related-target reconciliation, generated references, report-only behavior, and idempotent retries. Extensive executor and Fullsend tests cover the authorization boundary and no-call-on-failure guarantees.

Sequence diagram for trusted triage action preflight

sequenceDiagram
    participant Executor
    participant TrustedInput
    participant Preflight
    participant Jira
    Executor->>Preflight: _preflight_plan(result, trusted_input)
    Preflight->>TrustedInput: _trusted_targets(trusted_input)
    TrustedInput-->>Preflight: primary and related issue keys
    loop each action
        Preflight->>Preflight: _validate_action(raw_action)
        Preflight->>Preflight: _resolve_action(raw_action, registry)
        alt target outside trusted scope
            Preflight-->>Executor: ActionError
        else remediation task
            Preflight->>TrustedInput: _existing_remediation(action, trusted_input)
            alt project or retry state invalid
                Preflight-->>Executor: ActionError
            end
        end
    end
    Preflight-->>Executor: authorized plan
    Executor->>Jira: execute reads and writes
Loading

Flow diagram for fail-closed triage plan authorization

flowchart TD
    A[Mutation plan] --> B[_preflight_plan]
    B --> C[_trusted_targets]
    C --> D{Report issue matches trusted issue?}
    D -- No --> X[Reject before Jira operations]
    D -- Yes --> E[Validate and resolve each action]
    E --> F{Targets and references authorized?}
    F -- No --> X
    F -- Yes --> G{Remediation project and retry state valid?}
    G -- No --> X
    G -- Yes --> H[Execute plan with Jira reads and writes]
Loading

File-Level Changes

Change Details Files
Preflight the complete mutation plan against trusted issue scope before any Jira interaction.
  • Validate the trusted primary issue key and require it to match the report.
  • Build the authorized target set only from documented triage relationships, existing remediation state, and approved sibling searches.
  • Validate every action target, link endpoint, remediation project, and generated reference before execution or retry handling.
  • Reject invalid retry markers and unresolved or absent persisted remediation state without attempting digest repair.
plugins/sdlc-workflow/scripts/execute-triage-security-actions.py
Make action references immutable and preserve safe retry and idempotency behavior.
  • Prevent reference rebinding while resolving references in plan order.
  • Keep generated remediation identities symbolic until creation while binding existing trusted remediation targets.
  • Restrict field and status retry suppression to the trusted primary issue.
  • Retain report-only execution without requiring mutation identity context.
plugins/sdlc-workflow/scripts/execute-triage-security-actions.py
Add regression and contract coverage for trusted-scope authorization.
  • Verify unauthorized targets fail before Jira reads or writes, including late actions and CLI execution.
  • Cover malformed identities, unrelated search purposes, project mismatches, reference rebinding, retry suppression, and digest-repair prevention.
  • Verify prefetched related targets remain authorized and preserve mutation ordering.
  • Update existing retry and Fullsend fixtures with explicit trusted identity and project context.
plugins/sdlc-workflow/scripts/test_execute_triage_security_actions.py
plugins/sdlc-workflow/scripts/test_triage_security_fullsend.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. This changes the authorization boundary for Jira mutations: an incorrectly trusted target or reference could let an action plan edit, comment on, link, or create remediation issues outside the runner's intended scope. Reverting would stop future executions but would not undo already-persisted Jira changes, and the boundary decision applies to every mutation plan from the moment it ships.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_validate_action(raw_action)
action = _resolve_action(raw_action, registry)
action_type = action["type"]
target_fields = ()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authorization fail-open for future action types. The target-authorization mapping here enumerates known action types explicitly and defaults to target_fields = (). Any action type not listed (e.g. a future mutating type added to _execute_action/_REQUIRED_ACTION_FIELDS) passes preflight with no target check, re-opening the exact scope-escape this PR closes. Because target_fields is a second, hand-maintained enumeration separate from _execute_action and _REQUIRED_ACTION_FIELDS, a new type can be wired for execution while silently skipping authorization. Prefer deriving the mutation-target fields from a single source of truth (e.g. the required-fields table) so a new mutating type fails closed unless its targets are declared.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[sdlc-workflow/verify-pr] Classified as code change request - sub-task TC-6649 created to address this feedback. The preflight target_fields enumeration can fail open for unlisted/future mutating action types; the sub-task tracks deriving the mapping from a single source of truth so new types fail closed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified in 7ed87c05 (TC-6649). ✅ Approving.

Target fields and required fields now derive from a single _ACTION_DEFINITIONS table, and preflight fails closed: any action type absent from the table (or with empty targets, other than report-only) raises ActionError before any Jira I/O — a newly-added mutating type can no longer slip through with an unchecked target. Project binding for remediation-task is folded into the same target loop. The original scope-escape stays closed.

Comment thread plugins/sdlc-workflow/scripts/execute-triage-security-actions.py
Comment thread plugins/sdlc-workflow/scripts/execute-triage-security-actions.py
Comment thread plugins/sdlc-workflow/scripts/execute-triage-security-actions.py
@mrizzi mrizzi added the ok-to-test Enable verify-pr execution on PRs from forks label Sep 30, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

🤖 Finished Verify Pr · ✅ Success · Started 10:07 AM UTC · Completed 10:22 AM UTC

Commit: 8e4d62e · View workflow run →

Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $6.60

@fullsend-ai-review

Copy link
Copy Markdown

[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review - Classified as suggestion - an advisory recommendation to have a human review the authorization-boundary change; it requests no specific code modification. No sub-task created.

@fullsend-ai-review

Copy link
Copy Markdown

Verification Report for TC-6646 (commit 8e4d62e)

Check Result Details
Review Feedback WARN 2 code change requests -> 2 review-feedback sub-tasks created; 3 suggestions (incl. 1 review body) replied, no action
Root-Cause Investigation DONE 2 skill-gap root-cause tasks created (both plan-feature phase)
Scope Containment PASS 3/3 task files changed; no out-of-scope or unimplemented files
Diff Size PASS +307/-11 across 3 files (~80% in tests); proportionate to a scope-hardening fix
Commit Traceability PASS Commit 8e4d62e references TC-6646 ("Implements TC-6646")
Sensitive Patterns PASS No secrets/credentials in added lines
CI Status PASS Script Unit Tests 3.11-3.14, Skill Lint, Plugin Validation, Sourcery all success; 0 failures/pending
Acceptance Criteria PASS 7 of 7 criteria met
Test Quality PASS No repetitive tests; all test functions documented; Eval Quality: N/A
Test Change Classification ADDITIVE +10 test functions, none removed; no assertions weakened
Verification Commands PASS CI-covered (pytest/claude/uvx not runnable in sandbox; base branch absent)

Overall: WARN

Two reviewer-flagged defects require follow-up code changes and were tracked as review-feedback sub-tasks (both blocking TC-6646):

  1. Authorization fail-open for unlisted action types (comment 4143377726) - _preflight_plan defaults target_fields to () for action types not in its hand-maintained enumeration, so a future mutating type could bypass target authorization. Fix: derive the mapping from a single source of truth and fail closed.
  2. Related-issue status-transition retry hard-fails (comment 4143383009) - narrowing _already_applied to the primary issue makes related-target transition retries re-execute and abort. Fix: tolerate an already-reached transition as a no-op.

Root-cause investigation classified both as universal, method-based skill gaps originating at the plan-feature phase; two root-cause tasks were created (Related to TC-6646) with eval-coverage propagation for evals/plan-feature/evals.json.

The three remaining comments are optional suggestions (architectural refactor of the preflight/execute lockstep, up-front trusted-input schema validation + a regex-hoist nit, and Sourcery's human-review advisory); none matched a documented .py-scoped convention, so none were upgraded. All correctness, security, scope, and CI checks pass. Overall: WARN (review feedback requires follow-up; no blocking failures). This report is informational - a human reviewer decides whether to merge.


This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9.

Derive required fields and mutation targets from shared action definitions. Reject missing or empty target declarations during whole-plan preflight, before Jira reads, writes, or retry processing.

Implements TC-6649

Assisted-by: Claude Code
@github-actions github-actions Bot removed the ok-to-test Enable verify-pr execution on PRs from forks label Sep 30, 2026
Implements TC-6650

Assisted-by: Claude Code
@mrizzi mrizzi added the ok-to-test Enable verify-pr execution on PRs from forks label Sep 30, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

🤖 Finished Verify Pr · ✅ Success · Started 10:48 AM UTC · Completed 10:59 AM UTC

Commit: 6370cec · View workflow run →

Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $5.48

@fullsend-ai-review

Copy link
Copy Markdown

Verification Report for TC-6646 (commit 6370cec)

Check Result Details
Review Feedback WARN 2 code change requests -> sub-tasks TC-6649, TC-6650 (already created in prior run; both verified fixed by reviewer). All 4 inline threads + 1 review body already classified; no new items this run.
Root-Cause Investigation N/A No new sub-tasks created this run (idempotent re-run); prior root-cause tasks TC-6651, TC-6652 already exist.
Scope Containment PASS All 3 PR files exactly match the task's Files to Modify; no out-of-scope or unimplemented files.
Diff Size PASS 473 lines across 3 files (455+/18-), proportionate to an authorization-boundary fix plus its mandated regression suite.
Commit Traceability PASS All 3 commits reference a Jira ID (TC-6646 primary; TC-6649/TC-6650 for in-scope review-feedback sub-tasks on this branch).
Sensitive Patterns PASS No secrets, credentials, keys, or sensitive config in any added line.
CI Status PASS All substantive checks green (Script Unit Tests 3.11-3.14, Skill Lint, Plugin Validation, Sourcery); only non-pass entry is verify-pr's own in-progress run.
Acceptance Criteria PASS 7 of 7 criteria satisfied by the diff.
Test Quality PASS Repetitive Test PASS, Test Documentation PASS, Eval Quality N/A.
Test Change Classification ADDITIVE ~15 new test functions; no removed tests/assertions, no added skips, no broadened mocks; existing edits are coverage-preserving fixture adaptations.
Verification Commands PASS The 5 task commands validated via CI proxies (all green); no eval-infra changes.

Overall: WARN

The implementation fully satisfies TC-6646's acceptance criteria: mutation-authorized triage-security plans are now bound to the trusted issue/triage scope via a whole-plan preflight (_preflight_plan) that rejects untrusted targets, references, link endpoints, and remediation projects before any Jira operation, using a fail-closed single-source _ACTION_DEFINITIONS mapping. Report-only behavior, related-issue triage, remediation references, digest ordering, and idempotent retries are preserved. All CI checks pass and the change is cleanly scoped to the three declared files. Review feedback produced two code-change requests, both already tracked as sub-tasks (TC-6649, TC-6650) and confirmed fixed by the reviewer in follow-up commits; two suggestions and one advisory review-body note were classified without action. This is an idempotent re-run at a later commit (6370cec): all review items were already classified and all sub-tasks/root-cause tasks already exist, so no new Jira writes were made. WARN reflects only the presence of tracked review feedback -- there are no correctness, security, or scope failures. Merge remains a human decision.


This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9.

@mrizzi
mrizzi merged commit 59f9ff4 into RHEcosystemAppEng:verify-pr-fullsend Sep 30, 2026
61 checks passed
@mrizzi
mrizzi deleted the TC-6646 branch September 30, 2026 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Enable verify-pr execution on PRs from forks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant