fix: distinguish secrecy vs integrity in filtered notice#2518
Conversation
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
|
@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
There was a problem hiding this comment.
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
IsSecrecyViolationtodifc.FilteredItemDetailand set it duringEvaluator.FilterCollection. - Update
buildDIFCFilteredNoticeto use a newdifcPolicyLabelhelper and change the user-visible prefix from[DIFC]to[Filtered]. - Extend tests to assert
IsSecrecyViolationbehavior 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.
| @@ -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, | |||
| } | |||
There was a problem hiding this comment.
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.
Fixes the integrity filtering problems reported in #2502.
Problem
The
buildDIFCFilteredNoticefunction 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:min-integrity" as the fix — which has no effect on secrecy restrictionsThe root issue is that
FilteredItemDetailonly stored aReasonstring, losing the structured information about why an item was filtered (secrecy vs. integrity).Changes
internal/difc/resource.goIsSecrecyViolation booltoFilteredItemDetailinternal/difc/evaluator.goIsSecrecyViolationinFilterCollectionbased onresult.SecrecyToAddinternal/server/difc_log.godifcPolicyLabelhelper; updatebuildDIFCFilteredNoticeto use "secrecy policy" / "integrity policy" / "access policy"; replace internal[DIFC]prefix with user-friendly[Filtered]internal/server/difc_log_test.goTestBuildDIFCFilteredNotice_{SecrecyViolation,IntegrityViolation,MixedViolations}andTestDifcPolicyLabelinternal/difc/evaluator_test.goIsSecrecyViolationassertions to existingFilteredItemsHaveReasonstestsExample — before vs after
Before (all filtering labelled as integrity, internal term exposed):
After (correct label for secrecy blocking, user-friendly prefix):
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.