Skip to content

VAULT-38846: Add attribution data to secret version metadata#206

Merged
mpalmi merged 13 commits intomainfrom
davidadeleon/VAULT-38846
Sep 9, 2025
Merged

VAULT-38846: Add attribution data to secret version metadata#206
mpalmi merged 13 commits intomainfrom
davidadeleon/VAULT-38846

Conversation

@davidadeleon
Copy link
Contributor

@davidadeleon davidadeleon commented Sep 4, 2025

Overview

This PR enhances secret version metadata to include attribution data. This allows operators and users who have access to the secret to be able to identify what operation caused a version of a secret along with a display_name and entity_id of who performed that operation.

Design of Change

A new attribution struct has been created that gets populated from the logical request. This struct gets passed as part of the version creation call which then has its field stored as part of the secret version metadata.

Related Issues/Pull Requests

N/A

Contributor Checklist

[ ] Add relevant docs to upstream Vault repository, or sufficient reasoning why docs won’t be added yet
My Docs PR Link
Example
[ ] Add output for any tests not ran in CI to the PR description (eg, acceptance tests)
[ ] Backwards compatible
[ ] Changelog entry added. See Updating the Changelog.

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've documented the impact of any changes to security controls.
    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@davidadeleon davidadeleon requested review from a team as code owners September 4, 2025 20:59
path_data.go Outdated
attributionWarning = append(attributionWarning, "no entity id found")
}

attr := &Attribution{
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the drop-in comment here but would it make sense to include client ID as well? Just especially thinking about non-entity clients, obviously it's the same as clientId for entities.

Copy link
Contributor

Choose a reason for hiding this comment

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

@VioletHynes we've got a separate Jira to track non-entity attribution (for root/batch tokens). We were thinking of providing the token issue time and token type to facilitate further lookup. ClientID might be the perfect substitute for that though. @davidadeleon and I are still deciding if we want to address that in a separate PR since the work is tracked elsewhere and basically an improvement to the actor/DisplayName for specific types of auth.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the extra context! I do think it makes sense to have clientId somewhere in the attribution, but if it's coming in a later PR then that's perfectly fine by me :)

Co-authored-by: Mike Palmiotto <mike@p4lm.io>
@mpalmi mpalmi self-requested a review September 9, 2025 19:18
@mpalmi mpalmi merged commit 9c73d01 into main Sep 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants