diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index 7a924a60..45fcf892 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -7,6 +7,21 @@ use serde_json::Value; use super::constants::{field_names, label_constants}; +/// Extract a resource number from a JSON item, returning the number as a string. +/// Returns "unknown" (with a log warning) if the number field is missing or invalid. +pub(crate) fn extract_resource_number(item: &Value, resource_type: &str, repo: &str) -> String { + match item.get("number").and_then(|v| v.as_u64()) { + Some(n) => n.to_string(), + None => { + crate::log_warn(&format!( + "{}:{} — missing or invalid 'number' field, using 'unknown'", + resource_type, repo + )); + "unknown".to_string() + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ScopeKind { All, diff --git a/guards/github-guard/rust-guard/src/labels/response_items.rs b/guards/github-guard/rust-guard/src/labels/response_items.rs index 6676bef2..24cea931 100644 --- a/guards/github-guard/rust-guard/src/labels/response_items.rs +++ b/guards/github-guard/rust-guard/src/labels/response_items.rs @@ -113,7 +113,7 @@ pub fn label_response_items( }; for item in items_to_process.iter() { - let number = item.get("number").and_then(|v| v.as_i64()).unwrap_or(0); + let number = extract_resource_number(item, "pr", &arg_repo_full); // Get repo info from the PR's base or head let repo_full_name = item @@ -205,7 +205,7 @@ pub fn label_response_items( let repo_private = repo_visibility_private_for_repo_id(&repo_full_name) .unwrap_or(default_repo_private); - let number = item.get("number").and_then(|v| v.as_i64()).unwrap_or(0); + let number = extract_resource_number(item, "issue", &repo_full_name); let integrity = issue_integrity( item, &repo_full_name, diff --git a/guards/github-guard/rust-guard/src/labels/response_paths.rs b/guards/github-guard/rust-guard/src/labels/response_paths.rs index ed927b32..b65b7df2 100644 --- a/guards/github-guard/rust-guard/src/labels/response_paths.rs +++ b/guards/github-guard/rust-guard/src/labels/response_paths.rs @@ -145,7 +145,7 @@ pub fn label_response_paths( let item_repo_private = repo_visibility_private_for_repo_id(repo_for_labels) .unwrap_or(default_repo_private); - let pr_number = item.get("number").and_then(|v| v.as_u64()).unwrap_or(0); + let pr_number = extract_resource_number(item, "pr", repo_for_labels); let integrity = pr_integrity(item, repo_for_labels, item_repo_private, is_forked, ctx); let path = make_item_path(items_path, i); @@ -224,7 +224,7 @@ pub fn label_response_paths( let item_repo_private = repo_visibility_private_for_repo_id(repo_for_labels) .unwrap_or(default_repo_private); - let issue_number = item.get("number").and_then(|v| v.as_u64()).unwrap_or(0); + let issue_number = extract_resource_number(item, "issue", repo_for_labels); let integrity = issue_integrity( item, repo_for_labels, diff --git a/internal/difc/evaluator.go b/internal/difc/evaluator.go index 4fbd5329..e7ba75d6 100644 --- a/internal/difc/evaluator.go +++ b/internal/difc/evaluator.go @@ -155,6 +155,70 @@ func (e *Evaluator) GetMode() EnforcementMode { return e.mode } +// formatIntegrityLevel converts a list of integrity tags into a human-readable +// integrity level description (e.g., "approved" instead of "[unapproved:all approved:all]"). +func formatIntegrityLevel(tags []Tag) string { + if len(tags) == 0 { + return "none" + } + // Find the highest integrity level mentioned in the tags + highest := "" + for _, tag := range tags { + s := string(tag) + // Strip scope suffix (e.g., "approved:all" → "approved") + if idx := strings.Index(s, ":"); idx > 0 { + s = s[:idx] + } + switch s { + case "merged": + return "\"merged\"" + case "approved": + highest = "\"approved\"" + case "unapproved": + if highest == "" { + highest = "\"unapproved\"" + } + } + } + if highest != "" { + return highest + } + return fmt.Sprintf("%v", tags) +} + +// formatSecrecyLevel converts a list of secrecy tags into a human-readable +// secrecy scope description (e.g., "private (org/repo)" instead of "[private:org/repo]"). +func formatSecrecyLevel(tags []Tag) string { + if len(tags) == 0 { + return "public" + } + + bestScope := "" + hasPrivate := false + + for _, tag := range tags { + s := string(tag) + if strings.HasPrefix(s, "private:") { + scope := strings.TrimPrefix(s, "private:") + if scope != "" && len(scope) > len(bestScope) { + bestScope = scope + } + continue + } + if s == "private" { + hasPrivate = true + } + } + + if bestScope != "" { + return fmt.Sprintf("private (%s)", bestScope) + } + if hasPrivate { + return "private" + } + return fmt.Sprintf("%v", tags) +} + // newEmptyEvaluationResult creates a new EvaluationResult with default initialization. // This helper centralizes the common pattern of creating an empty result with AccessAllow decision // and empty tag slices, reducing duplication across evaluation functions. @@ -249,8 +313,8 @@ func (e *Evaluator) evaluateRead( result.Decision = AccessDeny result.IntegrityToDrop = integrityMissingTags result.Reason = fmt.Sprintf("Resource '%s' has lower integrity than agent requires. "+ - "Agent would need to drop integrity tags %v to trust this resource.", - resource.Description, integrityMissingTags) + "The agent cannot read data with integrity below %s.", + resource.Description, formatIntegrityLevel(integrityMissingTags)) return result } @@ -259,8 +323,8 @@ func (e *Evaluator) evaluateRead( result.Decision = AccessDeny result.SecrecyToAdd = secrecyExtraTags result.Reason = fmt.Sprintf("Resource '%s' has secrecy requirements that agent doesn't meet. "+ - "Agent would need to add secrecy tags %v to read this resource.", - resource.Description, secrecyExtraTags) + "The agent is not authorized to access %s-scoped data.", + resource.Description, formatSecrecyLevel(secrecyExtraTags)) return result } @@ -287,8 +351,8 @@ func (e *Evaluator) evaluateWrite( result.Decision = AccessDeny result.IntegrityToDrop = missingTags result.Reason = fmt.Sprintf("Agent lacks required integrity to write to '%s'. "+ - "Resource requires integrity tags %v that agent doesn't have.", - resource.Description, missingTags) + "The agent's integrity level is insufficient; it needs %s integrity.", + resource.Description, formatIntegrityLevel(missingTags)) return result } @@ -300,9 +364,9 @@ func (e *Evaluator) evaluateWrite( logEvaluator.Printf("Write denied: secrecy check failed, extraTags=%v", extraTags) result.Decision = AccessDeny result.SecrecyToAdd = extraTags - result.Reason = fmt.Sprintf("Agent has secrecy tags %v that cannot flow to '%s'. "+ - "Resource would need these secrecy requirements to accept the write.", - extraTags, resource.Description) + result.Reason = fmt.Sprintf("Agent carries %s-scoped data that cannot be written to '%s' due to secrecy constraints. "+ + "The target resource is not authorized to receive this sensitive data.", + formatSecrecyLevel(extraTags), resource.Description) return result } diff --git a/internal/difc/labels.go b/internal/difc/labels.go index 2a11ffdd..8d5ac740 100644 --- a/internal/difc/labels.go +++ b/internal/difc/labels.go @@ -373,21 +373,18 @@ func (e *ViolationError) Error() string { if e.Type == SecrecyViolation { msg = fmt.Sprintf("Secrecy violation for resource '%s': ", e.Resource) if len(e.ExtraTags) > 0 { - msg += fmt.Sprintf("agent has secrecy tags %v that cannot flow to resource. ", e.ExtraTags) - msg += "Remediation: remove these tags from agent's secrecy label or add them to the resource's secrecy requirements." + msg += fmt.Sprintf("the agent is not authorized to access data with secrecy level %s.", formatSecrecyLevel(e.ExtraTags)) } } else { if e.IsWrite { msg = fmt.Sprintf("Integrity violation for write to resource '%s': ", e.Resource) if len(e.MissingTags) > 0 { - msg += fmt.Sprintf("agent is missing required integrity tags %v. ", e.MissingTags) - msg += fmt.Sprintf("Remediation: agent must gain integrity tags %v to write to this resource.", e.MissingTags) + msg += fmt.Sprintf("the agent's integrity level is insufficient; it needs %s integrity.", formatIntegrityLevel(e.MissingTags)) } } else { msg = fmt.Sprintf("Integrity violation for read from resource '%s': ", e.Resource) if len(e.MissingTags) > 0 { - msg += fmt.Sprintf("resource is missing integrity tags %v that agent requires. ", e.MissingTags) - msg += fmt.Sprintf("Remediation: agent should drop integrity tags %v to trust this resource, or verify resource has higher integrity.", e.MissingTags) + msg += fmt.Sprintf("the agent cannot read data with integrity below %s.", formatIntegrityLevel(e.MissingTags)) } } } diff --git a/internal/difc/labels_test.go b/internal/difc/labels_test.go index f9fd2ed4..d3968d4b 100644 --- a/internal/difc/labels_test.go +++ b/internal/difc/labels_test.go @@ -507,9 +507,7 @@ func TestViolationError_Error(t *testing.T) { wantContains: []string{ "Secrecy violation", "classified-doc", - "[secret top-secret]", - "cannot flow to resource", - "Remediation:", + "not authorized to access", }, }, { @@ -523,8 +521,8 @@ func TestViolationError_Error(t *testing.T) { "Secrecy violation", "public-endpoint", }, - // No tag list or remediation when ExtraTags is empty - wantAbsent: []string{"cannot flow to resource"}, + // No tag list when ExtraTags is empty + wantAbsent: []string{"not authorized"}, }, { name: "integrity write violation with missing tags", @@ -537,8 +535,7 @@ func TestViolationError_Error(t *testing.T) { wantContains: []string{ "Integrity violation for write", "prod-db", - "missing required integrity tags", - "Remediation:", + "integrity level is insufficient", "production", "verified", }, @@ -555,7 +552,7 @@ func TestViolationError_Error(t *testing.T) { "Integrity violation for write", "prod-db", }, - wantAbsent: []string{"missing required integrity tags"}, + wantAbsent: []string{"integrity level is insufficient"}, }, { name: "integrity read violation with missing tags", @@ -568,8 +565,7 @@ func TestViolationError_Error(t *testing.T) { wantContains: []string{ "Integrity violation for read", "trusted-source", - "missing integrity tags", - "Remediation:", + "cannot read data with integrity below", "certified", }, }, @@ -585,7 +581,7 @@ func TestViolationError_Error(t *testing.T) { "Integrity violation for read", "trusted-source", }, - wantAbsent: []string{"missing integrity tags"}, + wantAbsent: []string{"cannot read data"}, }, }