Skip to content

fix: distinguish secrecy vs integrity in filtered notice#2518

Merged
lpcox merged 2 commits intomainfrom
copilot/fix-integrity-filtering-problems
Mar 25, 2026
Merged

fix: distinguish secrecy vs integrity in filtered notice#2518
lpcox merged 2 commits intomainfrom
copilot/fix-integrity-filtering-problems

Conversation

Copy link
Contributor

Copilot AI commented Mar 25, 2026

Fixes the integrity filtering problems reported in #2502.

Problem

The buildDIFCFilteredNotice function used "integrity policy" as a blanket term for all filtering — even when items were blocked for secrecy reasons. This caused the audit detection pipeline to:

  1. Label secrecy blocks as "Integrity filter blocked" (wrong type)
  2. Report "don't meet the GitHub integrity level" (wrong dimension)
  3. Suggest "lower min-integrity" as the fix — which has no effect on secrecy restrictions

The root issue is that FilteredItemDetail only stored a Reason string, losing the structured information about why an item was filtered (secrecy vs. integrity).

Changes

File Change
internal/difc/resource.go Add IsSecrecyViolation bool to FilteredItemDetail
internal/difc/evaluator.go Set IsSecrecyViolation in FilterCollection based on result.SecrecyToAdd
internal/server/difc_log.go Add difcPolicyLabel helper; update buildDIFCFilteredNotice to use "secrecy policy" / "integrity policy" / "access policy"; replace internal [DIFC] prefix with user-friendly [Filtered]
internal/server/difc_log_test.go Add TestBuildDIFCFilteredNotice_{SecrecyViolation,IntegrityViolation,MixedViolations} and TestDifcPolicyLabel
internal/difc/evaluator_test.go Add IsSecrecyViolation assertions to existing FilteredItemsHaveReasons tests

Example — before vs after

Before (all filtering labelled as integrity, internal term exposed):

[DIFC] 2 item(s) in this response were removed by integrity policy and are not shown:
resource:actions_get (has secrecy requirements that agent doesn't meet ...);
resource:get_job_logs (has secrecy requirements that agent doesn't meet ...)

After (correct label for secrecy blocking, user-friendly prefix):

[Filtered] 2 item(s) in this response were removed by secrecy policy and are not shown:
resource:actions_get (has secrecy requirements that agent doesn't meet ...);
resource:get_job_logs (has secrecy requirements that agent doesn't meet ...)

With this fix, downstream consumers (the audit detection pipeline, agents, operators) can accurately identify the type of violation and receive correct remediation guidance — without exposure to internal implementation terminology.


⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

The buildDIFCFilteredNotice function previously used "integrity policy"
as a blanket term for all DIFC filtering, even when items were blocked
for secrecy reasons. This caused the audit detection pipeline to
incorrectly label secrecy blocks as "Integrity filter blocked" and
suggest "lower min-integrity" as the fix - which doesn't help for
secrecy issues.

Changes:
- Add IsSecrecyViolation bool to FilteredItemDetail in resource.go
- Set IsSecrecyViolation in FilterCollection (evaluator.go) based on
  whether result.SecrecyToAdd is non-empty
- Add difcPolicyLabel helper that returns "secrecy policy",
  "integrity policy", or "DIFC policy" (mixed) based on violation types
- Update buildDIFCFilteredNotice to use the correct policy label
- Add tests: TestBuildDIFCFilteredNotice_{SecrecyViolation,
  IntegrityViolation,MixedViolations}, TestDifcPolicyLabel, and
  IsSecrecyViolation assertions in evaluator_test.go

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/3365235e-4348-4d27-9683-66b3e09c9c7d
@lpcox
Copy link
Collaborator

lpcox commented Mar 25, 2026

@copilot we should not expose the term DIFC to users.

Replace [DIFC] prefix with [Filtered] and "DIFC policy" with
"access policy" in the notice text surfaced to agents and users.

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/727d8f8a-a2ac-45e0-aef7-b2a9fd5ff428
Copy link
Contributor Author

Copilot AI commented Mar 25, 2026

@copilot we should not expose the term DIFC to users.

Done in 8ecdf2c. Replaced the user-facing [DIFC] prefix with [Filtered] and "DIFC policy" with "access policy" in the notice text. Internal log entries (file/stderr) still use [DIFC-FILTERED] since those are for operators, not end-users.

Copilot AI changed the title fix: distinguish secrecy vs integrity in DIFC filtered notice fix: distinguish secrecy vs integrity in filtered notice Mar 25, 2026
@lpcox lpcox marked this pull request as ready for review March 25, 2026 16:43
Copilot AI review requested due to automatic review settings March 25, 2026 16:43
@lpcox lpcox merged commit 628dc32 into main Mar 25, 2026
20 checks passed
@lpcox lpcox deleted the copilot/fix-integrity-filtering-problems branch March 25, 2026 16:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes DIFC filtered notices so they correctly distinguish secrecy vs integrity filtering (and uses a neutral “access policy” label when both occur), improving downstream audit/detection guidance and avoiding misleading remediation suggestions.

Changes:

  • Add IsSecrecyViolation to difc.FilteredItemDetail and set it during Evaluator.FilterCollection.
  • Update buildDIFCFilteredNotice to use a new difcPolicyLabel helper and change the user-visible prefix from [DIFC] to [Filtered].
  • Extend tests to assert IsSecrecyViolation behavior and validate notice labeling for secrecy, integrity, and mixed cases.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/difc/resource.go Extends FilteredItemDetail with IsSecrecyViolation to preserve filtering dimension.
internal/difc/evaluator.go Populates IsSecrecyViolation based on whether the denial was due to secrecy (SecrecyToAdd).
internal/server/difc_log.go Updates filtered notice text/prefix and adds difcPolicyLabel to select secrecy/integrity/access wording.
internal/server/difc_log_test.go Updates existing notice prefix assertions and adds new tests for secrecy/integrity/mixed labeling.
internal/difc/evaluator_test.go Adds assertions ensuring filtered items correctly set IsSecrecyViolation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 17 to +35
@@ -29,7 +30,26 @@ func newTestFilteredItem(data map[string]interface{}, description, reason string
Data: data,
Labels: labels,
},
Reason: reason,
Reason: reason,
IsSecrecyViolation: len(secrecyTags) > 0,
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

newTestFilteredItem sets IsSecrecyViolation based on whether secrecyTags is non-empty, but in the main codepath (Evaluator.FilterCollection) the flag is set based on whether a secrecy violation occurred (len(result.SecrecyToAdd) > 0). An item can legitimately have secrecy tags while being filtered for integrity reasons (or have secrecy-related reason text without tags in these tests), so this inference can produce incorrect IsSecrecyViolation values and make tests brittle/misleading. Consider either (a) passing an explicit isSecrecyViolation parameter to the helper, (b) deriving it from the same condition the production code uses for filtered items in these tests, or (c) leaving it unset in helpers that aren’t specifically constructing filtered-by-secrecy cases.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants