Skip to content

Commit b5e4572

Browse files
zwickCopilot
andcommitted
Omit unverified parent under lockdown instead of redacting title
Align issue_read get parent enrichment with the codebase's existing lockdown patterns: rather than introducing a third, redaction-with-sentinel behavior, omit the whole parent reference when its (possibly cross-repo) content cannot be verified safe. This mirrors how unsafe comments, sub-issues, and PR reviews are filtered out. has_parent stays true so an agent can still route to get_parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a8ed349 commit b5e4572

3 files changed

Lines changed: 20 additions & 25 deletions

File tree

pkg/github/issues.go

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -777,14 +777,10 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
777777
return MarshalledTextResult(minimalIssue), nil
778778
}
779779

780-
// lockdownRedactedTitle replaces a related issue's title when lockdown mode is enabled and the
781-
// related content cannot be verified as safe. Numeric/structural fields stay intact.
782-
const lockdownRedactedTitle = "[redacted - not from a trusted source]"
783-
784780
// applyIssueReadEnrichment populates the hierarchy relationship signals (has_parent/has_children,
785781
// parent, sub_issues_summary) and field_values onto the minimal issue. In lockdown mode the parent
786-
// title is redacted unless the parent content can be verified as safe; counts and booleans are pure
787-
// numbers and are always safe to surface.
782+
// reference is omitted unless the parent content can be verified as safe; has_parent and the numeric
783+
// counts are structural routing signals and are always safe to surface.
788784
func applyIssueReadEnrichment(ctx context.Context, minimalIssue *MinimalIssue, enrichment *issueReadEnrichment, cache *lockdown.RepoAccessCache, lockdownMode bool) {
789785
if enrichment == nil {
790786
return
@@ -793,12 +789,15 @@ func applyIssueReadEnrichment(ctx context.Context, minimalIssue *MinimalIssue, e
793789
minimalIssue.FieldValues = enrichment.FieldValues
794790

795791
if parent := enrichment.Parent; parent != nil {
796-
ref := parent.Ref
797-
if lockdownMode && !isSafeParentContent(ctx, cache, parent) {
798-
ref.Title = lockdownRedactedTitle
799-
}
800-
minimalIssue.Parent = &ref
801792
minimalIssue.HasParent = true
793+
// Surface the parent reference only when it is safe to expose. Under lockdown an
794+
// unverified (possibly cross-repo) parent is omitted entirely, mirroring how unsafe
795+
// comments and sub-issues are filtered out. has_parent still routes an agent to
796+
// get_parent if it needs to follow up.
797+
if !lockdownMode || isSafeParentContent(ctx, cache, parent) {
798+
ref := parent.Ref
799+
minimalIssue.Parent = &ref
800+
}
802801
}
803802

804803
if enrichment.SubIssuesSummary.Total > 0 {
@@ -808,9 +807,9 @@ func applyIssueReadEnrichment(ctx context.Context, minimalIssue *MinimalIssue, e
808807
}
809808
}
810809

811-
// isSafeParentContent reports whether the parent issue's title can be exposed under lockdown mode.
810+
// isSafeParentContent reports whether the parent issue reference can be exposed under lockdown mode.
812811
// It fails closed: any inability to positively verify safe content (missing cache, missing author,
813-
// unparseable repository, or a lookup error) results in redaction.
812+
// unparseable repository, or a lookup error) results in the parent reference being omitted.
814813
func isSafeParentContent(ctx context.Context, cache *lockdown.RepoAccessCache, parent *issueReadParent) bool {
815814
if cache == nil || parent.AuthorLogin == "" {
816815
return false

pkg/github/issues_test.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -714,8 +714,9 @@ func Test_GetIssue_HierarchyEnrichment_Lockdown(t *testing.T) {
714714
// In lockdown mode the issue's own author must be verified as safe (mirrors the existing
715715
// REST lockdown gate). The repo-access cache performs push-access checks against its own
716716
// REST client: the issue author ("author") has write access, while the parent author
717-
// ("parentauthor") only has read access and so cannot be verified as safe. The parent title
718-
// is therefore redacted while the numeric/structural fields remain intact.
717+
// ("parentauthor") only has read access and so cannot be verified as safe. The parent
718+
// reference is therefore omitted entirely, while has_parent stays true so an agent can
719+
// still route to get_parent.
719720
restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
720721
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue),
721722
})
@@ -757,14 +758,8 @@ func Test_GetIssue_HierarchyEnrichment_Lockdown(t *testing.T) {
757758
var returnedIssue MinimalIssue
758759
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedIssue))
759760

760-
require.NotNil(t, returnedIssue.Parent)
761-
assert.True(t, returnedIssue.HasParent)
762-
assert.Equal(t, lockdownRedactedTitle, returnedIssue.Parent.Title, "parent title should be redacted under lockdown")
763-
// Numeric/structural fields remain intact even when redacted.
764-
assert.Equal(t, 2820, returnedIssue.Parent.Number)
765-
assert.Equal(t, "OPEN", returnedIssue.Parent.State)
766-
assert.Equal(t, "owner/repo", returnedIssue.Parent.Repository)
767-
assert.Equal(t, "https://github.com/owner/repo/issues/2820", returnedIssue.Parent.URL)
761+
require.Nil(t, returnedIssue.Parent, "parent reference should be omitted under lockdown when it cannot be verified safe")
762+
assert.True(t, returnedIssue.HasParent, "has_parent should still be true so agents can route to get_parent")
768763
}
769764

770765
func Test_GetIssue_HierarchyEnrichment_QueryFailureReturnsBaseIssue(t *testing.T) {

pkg/github/minimal_types.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,9 @@ type MinimalIssue struct {
344344
FieldValues []MinimalFieldValue `json:"field_values,omitempty"`
345345

346346
// Hierarchy relationship signals. HasParent and HasChildren are always emitted
347-
// (an explicit false is itself a useful routing signal); Parent and SubIssuesSummary
348-
// are only populated when the relationship exists.
347+
// (an explicit false is itself a useful routing signal); SubIssuesSummary is populated
348+
// when children exist, and Parent when a parent exists and may be surfaced (under lockdown
349+
// an unverified parent reference is omitted while HasParent stays true).
349350
HasParent bool `json:"has_parent"`
350351
HasChildren bool `json:"has_children"`
351352
Parent *MinimalIssueRef `json:"parent,omitempty"`

0 commit comments

Comments
 (0)