fix: improve DIFC error messages and replace issue:#0 sentinel#2202
fix: improve DIFC error messages and replace issue:#0 sentinel#2202
Conversation
Guard improvements (Rust): - Replace unwrap_or(0) with extract_resource_number() helper that returns "unknown" instead of "0" for missing/invalid number fields - Log warnings when number field is missing (helps diagnose synthetic issue:#0 entries reported in DIFC analysis discussions) - Applies to both issue and PR number extraction in response_items.rs and response_paths.rs Error message improvements (Go): - Replace technical "Agent would need to drop integrity tags [X]" with human-readable "The agent cannot read data with integrity below X" - Replace "Agent would need to add secrecy tags" with "The agent is not authorized to access private-scoped data" - Add formatIntegrityLevel() that converts tag lists to named levels (e.g., [unapproved:all approved:all] → "approved") - Add formatSecrecyLevel() that converts tags to scope descriptions (e.g., [private:org/repo] → "private (org/repo)") - Update ViolationError.Error() with consistent plain-language messages - Remove verbose "Remediation:" hints from user-facing errors Addresses: github/gh-aw#21824, github/gh-aw#21866 (recommendation 5) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves DIFC denial/violation messages to be more human-readable and replaces the synthetic #0 sentinel in GitHub guard resource descriptions with #unknown when an issue/PR number is missing or invalid.
Changes:
- Added DIFC formatting helpers to render integrity/secrecy tag sets as simplified “levels”/scopes and updated evaluator + ViolationError messages accordingly.
- Updated Go unit tests to assert against the new error wording.
- Added a Rust helper to extract issue/PR numbers safely and updated response labeling to use
#unknowninstead of#0.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/difc/evaluator.go | Adds formatting helpers and updates read/write denial Reason strings to be more user-friendly. |
| internal/difc/labels.go | Updates ViolationError.Error() to use the new human-readable formatting. |
| internal/difc/labels_test.go | Updates expectations to match new error messages. |
| guards/github-guard/rust-guard/src/labels/helpers.rs | Introduces extract_resource_number() that returns "unknown" with a warning on missing/invalid numbers. |
| guards/github-guard/rust-guard/src/labels/response_items.rs | Switches PR/issue number extraction to the new helper for item-based labeling. |
| guards/github-guard/rust-guard/src/labels/response_paths.rs | Switches PR/issue number extraction to the new helper for path-based labeling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/difc/evaluator.go
Outdated
| result.Reason = fmt.Sprintf("Agent has secrecy tags %v that cannot flow to '%s'. "+ | ||
| "Resource would need these secrecy requirements to accept the write.", | ||
| "The agent carries sensitive data that the target resource is not authorized to receive.", | ||
| extraTags, resource.Description) |
There was a problem hiding this comment.
This denial reason still includes the low-level tag list and the target resource in the same sentence ("Agent has secrecy tags %v that cannot flow..."). In the PR description, the intended user-facing message for this case appears to be the simplified sentence only. If the goal is to avoid confusing non-experts, consider dropping the tag list from Reason (or moving detailed tags to debug logs / the detailed error output).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/cfe43072-48d3-4ed2-bad2-4583d326138a
…2205) The write-secrecy denial `Reason` exposed raw internal tag slices (e.g. `[private:owner/repo]`) in the user-facing message, inconsistent with read-denial messages which already use `formatSecrecyLevel`/`formatIntegrityLevel`. ## Changes - **`internal/difc/evaluator.go`:** Replace `%v extraTags` with `formatSecrecyLevel(extraTags)` in the write-secrecy denial `Reason`, producing e.g. `"private (owner/repo)"` instead of `[private:owner/repo]`. **Before:** ``` Agent has secrecy tags [private:owner/repo] that cannot flow to 'public-repo'. The agent carries sensitive data that the target resource is not authorized to receive. ``` **After:** ``` Agent carries private (owner/repo)-scoped data that cannot be written to 'public-repo' due to secrecy constraints. The target resource is not authorized to receive this sensitive data. ``` Raw tag details remain available in the debug log (`extraTags=%v`) and in `FormatViolationError`'s "Current Agent Labels" output. <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)
Summary
Improves DIFC error messaging for non-expert users and fixes the synthetic
issue:#0resource descriptions reported in gh-aw#21866 (recommendation 5).Addresses gh-aw#21824 — confusing "Agent would need to drop integrity tags" messages.
Changes
Guard (Rust) — Replace
#0sentinel with#unknownextract_resource_number()helper inhelpers.rsreturns"unknown"(withlog_warn) when thenumberfield is missing or invalidunwrap_or(0)in bothresponse_items.rsandresponse_paths.rsfor issues and PRsissue:github/gh-aw#0→ After:issue:github/gh-aw#unknownDIFC Evaluator (Go) — Human-readable error messages
New helper functions
formatIntegrityLevel()— converts tag lists to named levels (e.g.,[unapproved:all approved:all]→"approved")formatSecrecyLevel()— converts tags to scope descriptions (e.g.,[private:org/repo]→"private (org/repo)")Test Results