Skip to content

hide token secret in personal access token read endpoints#4826

Open
saddamr3e wants to merge 2 commits into
dragonflyoss:mainfrom
saddamr3e:pat-hide-token-read
Open

hide token secret in personal access token read endpoints#4826
saddamr3e wants to merge 2 commits into
dragonflyoss:mainfrom
saddamr3e:pat-hide-token-read

Conversation

@saddamr3e

Copy link
Copy Markdown
Contributor

Description

The personal access token read endpoints return the full models.PersonalAccessToken, and its Token field is tagged json:"token", so the secret bearer value is serialized in the response.

  1. GetPersonalAccessToken (GET /api/v1/personal-access-tokens/{id}) and GetPersonalAccessTokens (GET /api/v1/personal-access-tokens) both marshal the stored token value.
  2. RBAC seeds every authenticated user with the guest role, which holds read on all API objects, so any low-privilege user can list these endpoints and read the secret token of every user, then reuse those tokens (job/cluster scopes) to act as the owner.

Cleared the Token field in both read service methods. The token stays write-once, returned only by the create response.

Related Issue

None filed; reporting the fix directly.

Motivation and Context

A token is a credential. Read paths reachable by guest-level users should expose token metadata (name, scopes, state, owner) but never the secret value itself.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

Signed-off-by: saddamr3e <saddamr3e@gmail.com>
@pmady

pmady commented Jun 20, 2026

Copy link
Copy Markdown

makes sense, tokens should only be visible on creation. one thing — the variable is named personalAccessToken (singular) in GetPersonalAccessTokens but its actually a slice, might be worth renaming to personalAccessTokens while youre in here. minor nit though, the fix itself lgtm.

Signed-off-by: saddamr3e <saddamr3e@gmail.com>
@saddamr3e

Copy link
Copy Markdown
Contributor Author

Good catch, renamed it to personalAccessTokens. Pushed.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.07%. Comparing base (c9e2e6d) to head (1765f1b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
manager/service/personal_access_token.go 0.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4826      +/-   ##
==========================================
- Coverage   28.08%   28.07%   -0.01%     
==========================================
  Files         232      232              
  Lines       23153    23159       +6     
==========================================
  Hits         6502     6502              
- Misses      16211    16217       +6     
  Partials      440      440              
Flag Coverage Δ
unittests 28.07% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
manager/service/personal_access_token.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

6 participants