Skip to content

Limit Vault service account token reads#7786

Merged
zroubalik merged 2 commits into
kedacore:mainfrom
zroubalik:fix-vault-token-read-limit-7783
May 27, 2026
Merged

Limit Vault service account token reads#7786
zroubalik merged 2 commits into
kedacore:mainfrom
zroubalik:fix-vault-token-read-limit-7783

Conversation

@zroubalik

@zroubalik zroubalik commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Limits projected service account token reads used by Vault Kubernetes authentication.

The resolver previously read the configured token path without a size bound. This rejects non-regular files and caps the token read before JWT validation.

Checklist

  • Tests have been added (if applicable)
  • Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #7783

@github-actions

github-actions Bot commented May 26, 2026

Copy link
Copy Markdown

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@keda-automation keda-automation requested review from a team May 26, 2026 14:38
@snyk-io

snyk-io Bot commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@zroubalik zroubalik marked this pull request as ready for review May 26, 2026 14:41
@zroubalik

zroubalik commented May 26, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member Author

/run-e2e hashicorp
Update: You can check the progress here

passed tests: 1 
Execution of tests/secret-providers/hashicorp_vault/hashicorp_vault_test.go, has passed after "two" attempts
failed tests: 0

@zroubalik zroubalik added the required:keda-v2.20 This is absolutely mandatory to bring along label May 26, 2026
@zroubalik zroubalik mentioned this pull request May 26, 2026
Closed
22 tasks
@zroubalik zroubalik requested a review from Copilot May 26, 2026 14:46
Copilot AI reviewed May 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the Vault Kubernetes authentication path by bounding reads of projected service account token files and rejecting non-regular files, preventing potential memory/IO abuse before JWT validation.

Changes:

  • Add a maximum size cap for projected service account token reads and enforce it before token validation.
  • Reject non-regular token paths (eg, directories).
  • Extend tests and adjust Vault handler test expectations to match the updated error behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/scaling/resolver/k8s_validator.go Adds bounded/validated file reading for projected SA tokens before JWT parsing.
pkg/scaling/resolver/k8s_validator_test.go Adds test coverage for oversize token files and non-regular token paths.
pkg/scaling/resolver/hashicorpvault_handler_test.go Updates expected error substring for missing token path.
CHANGELOG.md Documents the fix in Unreleased fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/scaling/resolver/k8s_validator.go Outdated
@zroubalik zroubalik requested a review from Copilot May 26, 2026 15:03
Copilot AI reviewed May 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

rickbrouwer
rickbrouwer reviewed May 26, 2026
View reviewed changes
Comment thread pkg/scaling/resolver/k8s_validator.go
JorTurFer
JorTurFer approved these changes May 26, 2026
View reviewed changes

@JorTurFer JorTurFer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why 1MB? just random value?

@zroubalik

zroubalik commented May 27, 2026

Copy link
Copy Markdown
Member Author

why 1MB? just random value?

Kinda yes, do you think it is not enough?

@rickbrouwer

rickbrouwer commented May 27, 2026

Copy link
Copy Markdown
Member

Personally, I think it is rather large. But better large than too small.
I think it is good, that we document this (logically).

I don't know if it can be made configurable (with a default of, say, 1MB), and whether that is desirable.

@wozniakjan

wozniakjan commented May 27, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

/run-e2e hashicorp
Update: You can check the progress here

passed tests: 1 
Execution of tests/secret-providers/hashicorp_vault/hashicorp_vault_test.go, has passed after "one" attempts
failed tests: 0

@wozniakjan wozniakjan enabled auto-merge (squash) May 27, 2026 07:06
@wozniakjan

wozniakjan commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Personally, I think it is rather large. But better large than too small.

I agree with both statements, when thinking about JWTs, I think of them in order of kb, but having larger limit imho doesn't hurt

I don't know if it can be made configurable (with a default of, say, 1MB), and whether that is desirable.

I think it's ok either way, unlikely anyone will want to change this up or down so keeping this as constant is good enough

@zroubalik zroubalik disabled auto-merge May 27, 2026 08:51
@zroubalik zroubalik enabled auto-merge (squash) May 27, 2026 08:51
@zroubalik zroubalik merged commit 4f74a95 into kedacore:main May 27, 2026
23 checks passed
shcherbak pushed a commit to shcherbak/keda that referenced this pull request Jun 3, 2026
@zroubalik @shcherbak
Limit Vault service account token reads (kedacore#7786)
38ea444 
* Limit Vault service account token reads

Signed-off-by: Zbynek Roubalik <[email protected]>

* Validate Vault token file after opening

Signed-off-by: Zbynek Roubalik <[email protected]>

---------

Signed-off-by: Zbynek Roubalik <[email protected]>
Signed-off-by: Yurii Shcherbak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rickbrouwer rickbrouwer rickbrouwer left review comments
Copilot code review Copilot Copilot left review comments
@wozniakjan wozniakjan wozniakjan approved these changes
@JorTurFer JorTurFer JorTurFer approved these changes

Assignees

No one assigned

Labels

required:keda-v2.20 This is absolutely mandatory to bring along

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Vault Kubernetes auth reads service account token files without size bounds

5 participants

@zroubalik @rickbrouwer @wozniakjan @JorTurFer