feat(api): structured WARN logging on authorization denials - #189
Conversation
…pped when context role is missing
…in/* gate is RequireRole
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…default role, other JWT enforcements, doc changes.
…-safe, add empty-JWKS test
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
wavehouse-docs | 0cadcc6 | May 27 2026, 06:07 PM |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Free Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAuthorization denials now use an injected request-context *slog.Logger and emit structured WARN logs with reason, observed/resolved roles, allowed roles, route (chi pattern preferred), method, and status; handlers, middleware, wiring, and tests updated. ChangesAuthorization Denial Logging & Structured Instrumentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request centralizes authorization denial handling across the API to improve system observability. By routing all denials through a unified helper, the system now emits structured logs that capture essential context about why a request was rejected, such as role resolution details and route patterns. This change facilitates faster troubleshooting of policy misconfigurations without requiring request reproduction. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances authorization denial handling by emitting structured WARN logs containing detailed context (such as the observed and resolved roles, allowed roles, route template, and HTTP method) and adds comprehensive unit tests to verify these logs. However, a critical compilation error was identified in internal/api/errors.go where slog.LogAttrs is incorrectly called as a package-level function instead of a method on slog.Default().
EricAndrechek
left a comment
There was a problem hiding this comment.
I like the idea behind it, and you did a good job trying to keep it DRY, but I don't think that this implementation is complete, and honestly is doing more harm than good to logging details in its current form.
Summary
Every authorization denial now flows through one shared helper that emits a structured
slogWARN, so a misconfigured role or policy is visible in logs without reproducing the request.writeAuthzDenied(internal/api/errors.go) is the single denial path for all gates. It classifies the failure — a present-but-invalid/expired token fails loud as 401 with the sanitized token reason, everything else is a403 — and derives a greppable
reason(role not in allowed roles,no role and no default_role configured, or the token error).logAuthzDeniedemits the"authorization denied"WARN withreason,role_observed(pre-default-resolution) vsrole_resolved,roles_allowed, the chiroutepattern (low-cardinality, no concrete path params),method, andstatus. slog escapes control chars, so the request-derived fields carry no log-injection risk.forbiddenForRolereturns the 403 body — verbose for the anonymous / no-default_rolecase, terse otherwise (doesn't enumerate roles)./v1/admin/*gate (router.go), the pipe gate (pipes.go), and the policy-evaluator paths (ingest.go,structured_query.go).roles_allowedis populated only where a flat allowlist exists (a pipe'sallowed_roles); the admin gate and evaluator paths passnil.Test plan
make ci(andgo test ./internal/api/) passes locallyrole not in allowed roles, the role, route/v1/admin/query, status 403default_rolelogsrole_observed=""androle_resolved=<default>reasoncarries the token errorroles_allowedequal to the pipe'sallowed_rolesRelated Issues
Part of #145: implements the "structured WARN on every denial" requirement from its Proposed Solution. Does not fully close #145; the broader role-gating enforcement is tracked there.