Skip to content

feat(core): add global audit config and add configured JWT claims to audit logs#3429

Open
jakedoublev wants to merge 20 commits intomainfrom
feat/audit-entity-config-DSPX-2918
Open

feat(core): add global audit config and add configured JWT claims to audit logs#3429
jakedoublev wants to merge 20 commits intomainfrom
feat/audit-entity-config-DSPX-2918

Conversation

@jakedoublev
Copy link
Copy Markdown
Contributor

@jakedoublev jakedoublev commented May 1, 2026

Proposed Changes

  • add global audit config and add configured JWT claims to audit logs with dotnotation paths to placement within the event object
  • uses custom struct tags for audit to define paths in the current schema that are reserved (immutable via configured JWT claim audit paths) or extensible (leaves are mutable) with unit tests
  • audit config validation happens on startup, so expensive Go reflection is avoided in audit logging at runtime
  • enhances token-handling IPC-layer middleware to avoid parsing raw bearer tokens twice (on ingress and over IPC), for example in a flow external request -> registered PEP -> IPC authorization.v2.GetDecision

Validated with this config:

audit:
  jwt_claim_mappings:
    - claim: preferred_username
      path: banana.kiwi.mango

Decision audit log during rewrap flow (note .banana.kiwi.mango):

{
  "time": "2026-05-01T14:35:31.065374-07:00",
  "level": "AUDIT",
  "msg": "decision",
  "audit": {
    "action": {
      "result": "failure",
      "type": "read"
    },
    "actor": {
      "attributes": [
        {
          "entitlements_relevant_to_decision": {}
        }
      ],
      "id": "jwtentity-1-clientid-opentdf"
    },
    "banana": {
      "kiwi": {
        "mango": "service-account-opentdf"
      }
    },
    "clientInfo": {
      "platform": "authorization.v2",
      "requestIP": "",
      "userAgent": "connect-go/1.19.1 (go1.26.2)"
    },
    "eventMetaData": {
      "fulfillable_obligation_value_fqns": [],
      "obligations_satisfied": true,
      "resource_decisions": [
        {
          "passed": false,
          "obligations_satisfied": true,
          "entitled": false,
          "resource_id": "rewrap-0",
          "data_rule_results": [
            {
              "passed": false,
              "resource_value_fqns": [
                "https://example.com/attr/attr1/value/value1"
              ],
              "attribute": {
                "id": "6a261d68-0899-4e17-bb2f-124abba7c09c",
                "rule": 2,
                "fqn": "https://example.com/attr/attr1"
              },
              "entitlement_failures": [
                {
                  "attribute_value": "https://example.com/attr/attr1/value/value1",
                  "action": "read"
                }
              ]
            }
          ],
          "required_obligation_value_fqns": null
        }
      ]
    },
    "object": {
      "attributes": {
        "assertions": null,
        "attrs": null,
        "permissions": null
      },
      "id": "jwtentity-1-clientid-opentdf-read",
      "name": "decisionRequest-read",
      "type": "entity_object"
    },
    "original": null,
    "requestID": "05a9040f-12b6-4768-8399-f97b58a671bb",
    "timestamp": "2026-05-01T14:35:31-07:00",
    "updated": null
  }
}

KAS Rewrap audit log (note .banana.kiwi.mango):

{
  "time": "2026-05-01T14:35:31.065701-07:00",
  "level": "AUDIT",
  "msg": "rewrap",
  "audit": {
    "action": {
      "result": "error",
      "type": "rewrap"
    },
    "actor": {
      "attributes": [],
      "id": ""
    },
    "banana": {
      "kiwi": {
        "mango": "service-account-opentdf"
      }
    },
    "clientInfo": {
      "platform": "kas",
      "requestIP": "",
      "userAgent": "connect-go/1.19.1 (go1.26.2)"
    },
    "eventMetaData": {
      "algorithm": "rsa:2048",
      "keyID": "r1",
      "policyBinding": "ZWZiNTBlNDA3MjgyYjQ4OWIxZTMwODA1MjYxYTU2YWNiMzAwZjhhNWQzODA0MDc3MmVhN2Y3YmFjYmU4NTVkYg==",
      "tdfFormat": "tdf3"
    },
    "object": {
      "attributes": {
        "assertions": [],
        "attrs": [
          "https://example.com/attr/attr1/value/value1"
        ],
        "permissions": []
      },
      "id": "ab63e64a-45a5-11f1-aded-da43b5bed1af",
      "name": "",
      "type": "key_object"
    },
    "original": null,
    "requestID": "25fc8dae-af84-4c3f-916c-d815adc2a252",
    "timestamp": "2026-05-01T14:35:31-07:00",
    "updated": null
  }
}

Summary by CodeRabbit

  • New Features

    • Audit enrichment: optional JWT claim → audit-field mappings (dot-notation) added to configs (disabled by default via comments).
  • Improvements

    • Audit config validated at startup and applied to namespace loggers; startup fails on invalid audit config.
    • IPC auth context rehydration improved to restore propagated access tokens for downstream use.
  • Tests

    • Expanded tests for JWT-to-audit mappings, audit schema/validation, dot-notation helpers, and IPC auth rehydration.

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 1, 2026

Warning

Rate limit exceeded

@jakedoublev has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 29 minutes and 30 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d2ad1f42-6be5-42b4-95ba-75bea35a5714

📥 Commits

Reviewing files that changed from the base of the PR and between e924b91 and 520fbca.

📒 Files selected for processing (2)
  • service/logger/audit/config.go
  • service/logger/audit/schema_test.go
📝 Walkthrough

Walkthrough

Adds an optional audit configuration and JWT-to-audit-field enrichment: config model and validation, schema for allowed audit paths, dot-notation helpers, logger wiring to store/apply audit config, claim extraction/normalization into audit events, IPC auth-context rehydration from propagated metadata, and related tests and YAML examples.

Changes

Audit, Auth, and Logging Feature

Layer / File(s) Summary
Config Model / YAML examples
service/logger/audit/config.go, service/pkg/config/config.go, opentdf-*.yaml
Adds audit config types (JWTClaimMapping, Config), Config.Validate(), integrates Config.Audit into main config, and adds commented jwt_claim_mappings examples to YAML files.
Audit Schema & Annotations
service/logger/audit/schema.go, service/logger/audit/schema_test.go, service/logger/audit/utils.go
Builds destination-path schema from EventObject via reflection, adds reserved/extensible tags and sentinel errors, implements validateClaimDestinationPath and overlap checks, updates EventObject tags and LogValue() to emit normalized map; tests added.
Dot-Notation Utils
service/internal/dotnotation/dotnotation.go, service/internal/dotnotation/dotnotation_test.go
New Get/Set for dot-path access into nested map[string]any, including reflective conversion of string-keyed maps and collision/path validation, with unit tests covering creation, conversion, collisions, nil root, and malformed paths.
Audit Enrichment Implementation
service/logger/audit/enrichment.go, service/logger/audit/contextServerInterceptor.go
Implements JWT-claim-based enrichment: read claims from token in context, normalize values, write into audit entry using dot-notation; interceptor factory now accepts *Logger.
Logger State & API
service/logger/audit/logger.go, service/logger/audit/logger_test.go
Logger stores cloned audit Config via ApplyConfig; With propagates cloned config; audit transaction logging now builds enriched entry via buildLogEntry; tests extended with JWT helpers and many mapping/reservation/cloning cases.
Auth Context Rehydration & IPC
service/internal/auth/authn.go, service/internal/auth/authn_ipc_metadata_interceptor_test.go, service/pkg/auth/context_auth.go, service/pkg/auth/context_auth_test.go
Adds rehydrateIPCAuthContext and raw-token extraction to reconstruct auth context from incoming IPC metadata by parsing JWT without verification; IPCUnaryServerInterceptor calls rehydration after reauth checks; tests updated to use signed JWTs and cover invalid-token error path and JWK preservation.
Startup / Server Wiring
service/pkg/server/start.go, service/pkg/server/services.go, service/pkg/server/services_test.go, service/internal/server/server.go, service/pkg/server/options.go
Applies audit config during Start (failing on invalid mappings), extracts namespace logger creation into buildNamespaceLogger which applies audit config and returns errors on invalid overrides, wires audit logger into RPC interceptors, and adds a namespace-level logger test.
Tests / Helpers / Minor docs
service/logger/audit/*_test.go, service/internal/*_test.go, service/pkg/*_test.go, service/logger/logger.go
Extensive test additions/updates across audit, dot-notation, and auth IPC; doWithLogger helper refactor; small doc comment JSON tag change for RequestID; YAML files annotated with commented mapping examples.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant IPC as IPC Interceptor
    participant AuthCtx as Auth Context
    participant Handler as Service Handler
    participant Audit as Audit Logger

    Client->>IPC: gRPC request (metadata: access_token / Authorization)
    IPC->>AuthCtx: ipcReauthCheck (route reauthorization)
    IPC->>AuthCtx: rehydrateIPCAuthContext (extract raw token)
    AuthCtx->>AuthCtx: parse JWT (no verification) -> store token + raw bytes
    IPC->>Handler: invoke handler with rehydrated context
    Handler->>Audit: emit audit event
    Audit->>AuthCtx: read claims from token in context
    Audit->>Audit: apply JWTClaimMappings (dot-notation Get/Set)
    Audit->>Audit: normalize values and log enriched audit entry
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • elizabethhealy
  • alkalescent

Poem

🐰 I hopped along a dotted trail,
JWT crumbs tucked in my tail,
Mapped the claims to fields so neat,
Kept reserved rooms tidy and sweet,
Now audit logs hum — a tiny tale.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main changes: adding global audit configuration and injecting configured JWT claims into audit logs.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/audit-entity-config-DSPX-2918

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 29 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 enhances the platform's audit logging capabilities by enabling the enrichment of audit logs with claims extracted from JWTs. It introduces a configurable mapping system that allows administrators to specify which JWT claims should be included in audit logs and where they should be placed within the log structure. This provides better traceability and context for audit events without requiring changes to the core business logic.

Highlights

  • Global Audit Configuration: Introduced a new global audit configuration section in the YAML configuration files, allowing for JWT claim mappings to be defined.
  • JWT Claim Enrichment: Implemented functionality to automatically extract and map JWT claims into the audit logs based on the new configuration, using a new dot-notation utility for nested map access.
  • Audit Logger Refactoring: Updated the audit logger to support configuration application and enrichment, ensuring that sensitive or reserved audit paths are protected.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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.


Logs flow deep with claims in tow, Mapping paths where secrets go. Dot notation guides the way, To keep our audit records gray.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions github-actions Bot added the size/m label May 1, 2026
Comment thread service/pkg/auth/context_auth.go Fixed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 190.834972ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 96.485976ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 414.131839ms
Throughput 241.47 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 46.476838767s
Average Latency 463.224956ms
Throughput 107.58 requests/second

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces audit log enrichment, allowing JWT claims to be mapped to specific audit log fields using a new dot notation utility. It also updates the authentication package to support token retrieval from gRPC metadata as a fallback. Feedback was provided to expand the list of reserved audit paths to include all core fields, ensuring that custom mappings cannot overwrite critical audit information and compromise log integrity.

Comment thread service/logger/audit/enrichment.go Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 189.143068ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 104.970643ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 416.641841ms
Throughput 240.01 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 45.872678532s
Average Latency 457.385967ms
Throughput 109.00 requests/second

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 183.672987ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 94.539533ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 398.613477ms
Throughput 250.87 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.149096924s
Average Latency 429.690425ms
Throughput 115.88 requests/second

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 192.167482ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 97.648358ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 402.821319ms
Throughput 248.25 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.012214514s
Average Latency 418.191071ms
Throughput 119.01 requests/second

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

X-Test Failure Report

opentdfplatformSKC5TO.dockerbuild
govulncheck-failure-8

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 170.404447ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 87.304526ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 450.828525ms
Throughput 221.81 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.806469622s
Average Latency 435.993746ms
Throughput 114.14 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
// We should probably return an error here?
l.ErrorContext(ctx, "invalid authContext")
if l != nil {
l.ErrorContext(ctx, "invalid authContext")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual logged message is just "invalid authContext", so I think this is not necessary?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 175.438321ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 87.751538ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 420.322265ms
Throughput 237.91 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 45.291381327s
Average Latency 450.386839ms
Throughput 110.40 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 195.345569ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 101.77468ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 440.762553ms
Throughput 226.88 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 45.715361556s
Average Latency 455.910229ms
Throughput 109.37 requests/second

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 150.629977ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 75.728051ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 398.930607ms
Throughput 250.67 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.547481868s
Average Latency 424.086922ms
Throughput 117.52 requests/second

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 141.47387ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 68.110417ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 339.167155ms
Throughput 294.84 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 33.78448445s
Average Latency 336.317411ms
Throughput 148.00 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Timestamp string `json:"timestamp"`
Original map[string]any `json:"original,omitempty" audit:"extensible"`
Updated map[string]any `json:"updated,omitempty" audit:"extensible"`
RequestID uuid.UUID `json:"requestID" audit:"reserved"`
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to align with the existing LogValue() implementation which actually drove the shape and syntax of the emitted logs, rather than marshaled JSON.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 196.038725ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 103.992301ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 401.202453ms
Throughput 249.25 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 43.600645327s
Average Latency 434.878797ms
Throughput 114.68 requests/second

@jakedoublev jakedoublev enabled auto-merge May 1, 2026 23:25
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
service/pkg/auth/context_auth.go (1)

46-57: ⚠️ Potential issue | 🟠 Major

Add defensive nil check for interface-typed logger to prevent potential panic.

Line 55's if l != nil check is insufficient in Go: a typed nil pointer assigned to the optionalErrorLogger interface will still evaluate as non-nil, allowing line 56's l.ErrorContext() call to panic. Either use a more robust check (e.g., reflection to verify the underlying pointer is nil) or pass a no-op logger implementation instead of nil.

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@service/internal/auth/authn.go`:
- Around line 515-518: The handler currently treats failures from
rehydrateIPCAuthContext as internal server errors; instead, convert those
authentication failures into an unauthenticated response by returning
connect.NewError with connect.CodeUnauthenticated when
rehydrateIPCAuthContext(nextCtx, a.logger) returns an error. Update the error
return in the block that calls rehydrateIPCAuthContext so it uses
CodeUnauthenticated and a clear auth-failure message (refer to the call site of
rehydrateIPCAuthContext in authn.go).

In `@service/logger/audit/enrichment.go`:
- Around line 50-56: ApplyConfig is allowing overlapping mapping paths (e.g.
"banana" vs "banana.kiwi.mango") which causes dotnotation.Set to fail at
runtime; add a validation step in Config.Validate() (or call from ApplyConfig)
that scans all mapping.Path values and rejects the config if any path is a
prefix of another path (or vice versa). Implement this by comparing normalized
path tokens for each pair of mappings (use mapping.Path and mapping.Claim to
construct clear error messages) and return a validation error instead of
accepting the config so the service fails fast rather than emitting per-request
enrichment errors from dotnotation.Set.

In `@service/logger/audit/schema.go`:
- Around line 114-124: The parseAuditTag function should fail fast on unknown
tag values: change parseAuditTag signature to return (bool, bool, error), make
the switch default return a descriptive error for unrecognized tag strings
(e.g., "unknown audit tag: ..."), update every call site in schema construction
code to handle and propagate that error (so schema building returns an error
instead of silently treating unknown tags as unset), and update any tests to
expect error propagation from parseAuditTag during schema construction.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ff57d941-f246-4516-97c9-9e60e4a9bf17

📥 Commits

Reviewing files that changed from the base of the PR and between 138f9f4 and 0811a3c.

📒 Files selected for processing (15)
  • service/internal/auth/authn.go
  • service/internal/auth/authn_ipc_metadata_interceptor_test.go
  • service/internal/dotnotation/dotnotation.go
  • service/internal/dotnotation/dotnotation_test.go
  • service/logger/audit/enrichment.go
  • service/logger/audit/logger.go
  • service/logger/audit/logger_test.go
  • service/logger/audit/schema.go
  • service/logger/audit/schema_test.go
  • service/logger/audit/utils.go
  • service/logger/logger.go
  • service/pkg/auth/context_auth.go
  • service/pkg/auth/context_auth_test.go
  • service/pkg/server/services.go
  • service/pkg/server/services_test.go

Comment thread service/internal/auth/authn.go
Comment thread service/logger/audit/enrichment.go
Comment thread service/logger/audit/schema.go Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

…ct unknown audit tags

Return connect.CodeUnauthenticated instead of CodeInternal when IPC
token rehydration fails, since a malformed token is an auth failure
not a server fault.

Fail fast on unrecognized audit struct tag values (e.g. a typo like
audit:"resreved") by returning an error from parseAuditTag, propagated
through schema construction to panic at startup.

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
Two mappings where one path is a prefix of another (e.g. "banana" and
"banana.kiwi.mango") would pass individual validation but cause
dotnotation.Set collisions at runtime. Detect this during
Config.Validate() so the service fails fast at startup instead of
emitting partially enriched audit logs per-request.

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 200.54575ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 102.382713ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 406.680274ms
Throughput 245.89 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 45.075428469s
Average Latency 449.051278ms
Throughput 110.93 requests/second

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
service/logger/audit/enrichment.go (1)

50-56: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject overlapping destination paths at config-validate time to prevent partial runtime enrichment.

Line 50 still handles dotnotation.Set failures per-request. If mappings overlap (e.g., banana and banana.kiwi.mango), enrichment can be partially applied at runtime. This should be rejected during Config.Validate() so startup fails fast.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/logger/audit/enrichment.go` around lines 50 - 56, Add a config-time
check in Config.Validate() that scans all audit JWT-to-path mappings and rejects
any overlapping destination paths (prefix/child relationships) so runtime calls
to dotnotation.Set cannot partially apply enrichment; implement this by
comparing each mapping.Path against others (or building a simple prefix trie)
and return a validation error that references the two conflicting mappings (use
mapping.Path and mapping.Claim to form the message). Keep the per-request
dotnotation.Set error handling but ensure overlapping cases are caught and cause
startup validation to fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@service/internal/auth/authn.go`:
- Around line 476-484: The IPC token extraction currently trims only exact-cased
prefixes using strings.TrimPrefix on authHeaders[0], which fails for
mixed/lowercase schemes; update the logic (in the function that reads md.Get and
returns the token) to perform case-insensitive scheme detection and removal:
read authHeaders[0] into a variable, lower-case a prefix slice (e.g.,
strings.ToLower(header[:n])) or use strings.HasPrefix on strings.ToLower(header)
to check for "bearer " and "dpop " and then return the original header with the
corresponding prefix length removed and trimmed; ensure you still handle both
"authorization" and "Authorization" via md.Get and return "" when header is
empty.

In `@service/logger/audit/schema.go`:
- Around line 166-177: Ensure the full dot-path syntax is validated before
allowing an extensible-node early success: before splitting/iterating (where
auditClaimDestinationSchema, segments, current, and current.extensible are used)
check for malformed paths like trailing dot or consecutive dots (e.g.,
strings.HasSuffix(path, ".") or strings.Contains(path, "..")) and return
ErrUnknownAuditPath if found; this prevents returning nil on the first unknown
segment when current.extensible would otherwise short-circuit and accept
malformed inputs.

---

Duplicate comments:
In `@service/logger/audit/enrichment.go`:
- Around line 50-56: Add a config-time check in Config.Validate() that scans all
audit JWT-to-path mappings and rejects any overlapping destination paths
(prefix/child relationships) so runtime calls to dotnotation.Set cannot
partially apply enrichment; implement this by comparing each mapping.Path
against others (or building a simple prefix trie) and return a validation error
that references the two conflicting mappings (use mapping.Path and mapping.Claim
to form the message). Keep the per-request dotnotation.Set error handling but
ensure overlapping cases are caught and cause startup validation to fail fast.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6798ee0e-a717-4ee0-918c-61800dd82ab7

📥 Commits

Reviewing files that changed from the base of the PR and between 0811a3c and 6e2e191.

📒 Files selected for processing (5)
  • service/internal/auth/authn.go
  • service/internal/auth/authn_ipc_metadata_interceptor_test.go
  • service/logger/audit/enrichment.go
  • service/logger/audit/schema.go
  • service/logger/audit/schema_test.go

Comment thread service/internal/auth/authn.go
Comment thread service/logger/audit/schema.go
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Return (auditFieldOptions, error) instead of (*auditFieldOptions, error)
to avoid nil-nil returns flagged by nilnil linter. A zero-value opts
(empty name) signals "skip this field."

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

X-Test Failure Report

opentdfplatformMVMW0X.dockerbuild
govulncheck-failure-8

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 212.180699ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 106.567833ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 407.840345ms
Throughput 245.19 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 44.444256517s
Average Latency 442.918348ms
Throughput 112.50 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 209.221271ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 108.359121ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 416.705541ms
Throughput 239.98 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 46.550101266s
Average Latency 463.803933ms
Throughput 107.41 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

@jakedoublev jakedoublev requested a review from strantalis May 4, 2026 16:31
Hoist empty-segment validation before the schema traversal loop so
malformed paths like "banana..mango" or "banana." are rejected even
when the first segment triggers an extensible early return.

Also remove redundant md.Get("Authorization") fallback in IPC token
extraction since gRPC metadata keys are case-insensitive.

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 165.254767ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 92.731298ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 408.787176ms
Throughput 244.63 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 44.975058789s
Average Latency 448.614898ms
Throughput 111.17 requests/second

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@service/internal/auth/authn.go`:
- Around line 445-468: rehydrateIPCAuthContext currently restores pkg/auth state
but does not re-establish audit.ActorID, so IPC hops lose ActorID even when the
JWT 'sub' is present; update rehydrateIPCAuthContext to extract the actor id
(e.g., from parsed.Subject or parsed claims) after successful jwt.Parse and call
audit.ContextWithActorID(ctx, actorID) before returning the context (preserve
existing ctxAuth.ContextWithAuthNInfo usage and use
ctxAuth.GetJWKFromContext(ctx, l) and the parsed token to derive the subject),
ensuring IPCMetadataClientInterceptor/checkToken behavior is preserved and
audit.ActorID is rehydrated for later audit events.

In `@service/logger/audit/config.go`:
- Around line 57-67: The current isPathPrefix(short, long []string) ignores
identical paths because it returns false when len(short) >= len(long); change
the logic so identical slices are treated as a prefix conflict: update
isPathPrefix to return true when lengths are equal and all elements match (or
alternatively change the initial check to only return false when len(short) >
len(long)), so two mappings with the same Path (same slice contents) are
detected as conflicts and can be rejected by the caller that validates mappings;
keep the element-by-element comparison loop and ensure callers of isPathPrefix
(the mapping validation code) will reject duplicate destinations rather than
allowing silent overwrite.

In `@service/logger/audit/enrichment.go`:
- Around line 121-124: Change the call that discards the parse error to capture
and log it: replace "opts, _ := parseAuditFieldOptions(field)" with "opts, err
:= parseAuditFieldOptions(field)" (or equivalent) and if err != nil emit a
descriptive log entry including the field identifier and err before continuing;
keep the existing behavior of continuing when opts.name == "" but ensure
parseAuditFieldOptions errors are logged for observability. Use the existing
package/logger utility available in this file (or the nearest logger) so the log
includes context (function in enrichment.go, the field value, and the error).

In `@service/logger/audit/schema_test.go`:
- Around line 80-112: Add a unit test to cover identical destination paths by
calling validateNoOverlappingPaths with two JWTClaimMapping entries that have
the same Path (e.g., both "eventMetaData.requester.sub") and assert the intended
behavior (either require.NoError if duplicates are allowed or
require.ErrorIs(..., ErrOverlappingAuditPaths) if duplicates should be
rejected); reference validateNoOverlappingPaths, JWTClaimMapping, and
ErrOverlappingAuditPaths to locate where to add the new t.Run case in
TestValidateNoOverlappingPaths.

In `@service/logger/audit/schema.go`:
- Around line 176-201: Remove the unreachable "return nil" that follows the for
loop (the one after iterating over segments) in this function: the loop always
returns on the last segment via the switch that checks child.reserved,
child.extensible, and child.isLeaf, so delete that final return to clarify
control flow; if you prefer a defensive artifact, replace it with a short
comment stating the return is unreachable instead of keeping dead code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2966a3ce-b3dd-40b3-ac1e-34db60e85549

📥 Commits

Reviewing files that changed from the base of the PR and between 6e2e191 and e924b91.

📒 Files selected for processing (5)
  • service/internal/auth/authn.go
  • service/logger/audit/config.go
  • service/logger/audit/enrichment.go
  • service/logger/audit/schema.go
  • service/logger/audit/schema_test.go

Comment thread service/internal/auth/authn.go
Comment thread service/logger/audit/config.go
Comment thread service/logger/audit/enrichment.go
Comment thread service/logger/audit/schema_test.go
Comment thread service/logger/audit/schema.go
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Two claims mapped to the same path would silently overwrite each other
at runtime. Detect this in validateNoOverlappingPaths and reject at
config time. Also refactored pair iteration to check each pair once.

Signed-off-by: jakedoublev <jake.vanvorhis@virtru.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 153.646403ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 77.553026ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 343.223023ms
Throughput 291.36 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 34.379451414s
Average Latency 342.685203ms
Throughput 145.44 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • tests-bdd

See the workflow run for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants