[log] Add debug logging to DIFC violation error formatting - #11106
Conversation
Add two logLabels debug logging calls to internal/difc/violations.go using the existing logLabels logger (declared in labels.go): - ViolationError.Detailed(): log entry with violation type/resource - FormatViolationError: log the early-return control-flow branch when access is allowed (no violation to format) No new logger declared; reuses existing difc:labels logger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds debug traceability to DIFC violation error formatting.
Changes:
- Logs detailed violation-message construction.
- Logs the allowed-access early return.
Show a summary per file
| File | Description |
|---|---|
internal/difc/violations.go |
Adds debug logs to violation formatting paths. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
internal/difc/violations.go:83
- This new branch log also uses the logger created in
labels.go, which gives it the unrelateddifc:labelsnamespace. This means users filtering on the expecteddifc:violationsnamespace cannot see the event. Per AGENTS.md:307-312 and 337-344, use a descriptive file-local logger created withlogger.ForFile()for these violation-formatting messages.
logLabels.Print("FormatViolationError: access allowed, no violation error to format")
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
|
|
||
| // Detailed returns a detailed error message with full context | ||
| func (e *ViolationError) Detailed() string { | ||
| logLabels.Printf("Detailed: building full context for %s violation, resource=%s", e.Type, e.Resource) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback |
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed in |
🔒 mcpg Read-Only Stress — gVisorSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: PASS B-detail: All 7 write tool calls returned MCP References: §31717105824
|
🔒 mcpg Read-Only Stress — defaultSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: PASS Part B enforcement detail: Gateway enforces read-only by registering only 22 read tools; write tools return gateway MCP error References: §31717105778
|
🔒 mcpg Read-Only Stress — docker-sbxSurface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Overall: PASS The gateway exposes exactly 22 read-only tools. All write tool calls return References: §31717105740
|
Summary
Adds debug logging calls to
internal/difc/violations.go, reusing the existinglogLabelslogger (declared ininternal/difc/labels.goasdifc:labels).Changes
ViolationError.Detailed(): logs entry with violation type and resource for traceability when building the full-context error message.FormatViolationError(): logs theAccessAllowearly-return control-flow branch, making it clear in debug output when no violation error needed to be formatted.Notes
logLabelscalls elsewhere, so the new calls reuse that logger and naming convention.internal/syncutil/ttl_cache.goandinternal/sanitize/*were skipped because adding aloggerimport there would create an import cycle (theloggerpackage itself importssyncutilandsanitize).config/expand.go,launcher/log_helpers.go,mcp/connection_methods.go,cmd/output.go,config/gateway_env.go, etc.) already had extensive existing logging calls.Validation
gofmt -l internal/difc/violations.go— cleango vet ./internal/difc/...— cleango test ./internal/difc/...— passesgo build ./...— succeedsgo test ./...— pre-existing integration test failures unrelated to this change (missingawmgbinary path in test harness, present before this change)