Repository navigation
fix(triage-security): bind action plans to trusted issue scope - #319
Conversation
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
Reviewer's GuideThe 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 preflightsequenceDiagram
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
Flow diagram for fail-closed triage plan authorizationflowchart 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]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
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.
| _validate_action(raw_action) | ||
| action = _resolve_action(raw_action, registry) | ||
| action_type = action["type"] | ||
| target_fields = () |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
[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.
There was a problem hiding this comment.
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.
|
🤖 Finished Verify Pr · ✅ Success · Started 10:07 AM UTC · Completed 10:22 AM UTC Commit: Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $6.60 |
|
[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. |
Verification Report for TC-6646 (commit 8e4d62e)
Overall: WARNTwo reviewer-flagged defects require follow-up code changes and were tracked as review-feedback sub-tasks (both blocking TC-6646):
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 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 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
Implements TC-6650 Assisted-by: Claude Code
|
🤖 Finished Verify Pr · ✅ Success · Started 10:48 AM UTC · Completed 10:59 AM UTC Commit: Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $5.48 |
Verification Report for TC-6646 (commit 6370cec)
Overall: WARNThe 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. |
59f9ff4
into
RHEcosystemAppEng:verify-pr-fullsend
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:
Implements TC-6646
Summary by Sourcery
Preflight triage security action plans against trusted issue scope and execution context before allowing Jira operations.
Bug Fixes:
Enhancements:
Tests: