Skip to content

runtimevar/hashivault: reject secret paths that escape the mount - #3764

Merged
vangent merged 1 commit into
google:masterfrom
herdiyana256:runtimevar-hashivault-path-traversal
Aug 3, 2026
Merged

runtimevar/hashivault: reject secret paths that escape the mount#3764
vangent merged 1 commit into
google:masterfrom
herdiyana256:runtimevar-hashivault-path-traversal

Conversation

@herdiyana256

@herdiyana256 herdiyana256 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

newWatcher (runtimevar/hashivault/hashivault.go) builds the literal Vault API request path with path.Join(mount, "data", secretPath) (engine v2) or path.Join(mount, secretPath) (v1). path.Join cleans .. segments, so a secretPath such as "../../sys/health" makes the actual HTTP request escape the intended KV mount entirely, confirmed against a fake Vault server, where secretPath = "a/../../../auth/token/create" produced a request to /v1/auth/token/create instead of /v1/secret/data/....

secretPath is exactly the parameter OpenVariable(client, secretPath, ...) is documented to take directly, and for hashivault:// URLs it's derived unescaped from the URL host+path (path.Join(u.Host, u.Path)). Vault's own authorization model is commonly path-prefix ACLs (e.g. scoped to secret/data/tenant-a/* for a specific tenant), so a caller that only controls the secret path, a plausible per-tenant identifier, can reach paths the deploying application never intended to expose.

Adds vaultPath, mirroring the fix already applied to the sibling secrets/hashivault package (#3763): it joins the prefix and secretPath and verifies the result is still inside that prefix, returning an error instead of opening the variable otherwise. Relative navigation that stays within the mount (e.g. "tenant-a/../tenant-b") still works; only escapes past that boundary are rejected.

newWatcher builds the literal Vault API request path with
path.Join(mount, "data", secretPath) (engine v2) or path.Join(mount,
secretPath) (v1). path.Join cleans ".." segments, so a secretPath such
as "../../sys/health" makes the actual HTTP request escape the
intended KV mount entirely -- confirmed against a fake Vault server,
where secretPath = "a/../../../auth/token/create" produced a request
to /v1/auth/token/create instead of /v1/secret/data/....

secretPath is exactly the parameter OpenVariable(client, secretPath,
...) is documented to take directly, and for hashivault:// URLs it's
derived unescaped from the URL host+path. Vault's own authorization
model is commonly path-prefix ACLs (e.g. scoped to
secret/data/tenant-a/* for a specific tenant), so a caller that only
controls the secret path -- a plausible per-tenant identifier -- can
reach paths the deploying application never intended to expose.

Adds vaultPath, mirroring the fix already applied to the sibling
secrets/hashivault package (google#3763): it joins the prefix and secretPath
and verifies the result is still inside that prefix, returning an
error instead of opening the variable otherwise. Relative navigation
that stays within the mount (e.g. "tenant-a/../tenant-b") still works;
only escapes past that boundary are rejected.
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.44%. Comparing base (7654671) to head (29afea5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3764      +/-   ##
==========================================
+ Coverage   75.39%   75.44%   +0.05%     
==========================================
  Files         104      104              
  Lines       14255    14263       +8     
==========================================
+ Hits        10747    10761      +14     
+ Misses       2770     2763       -7     
- Partials      738      739       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@vangent
vangent merged commit 35f55f2 into google:master Aug 3, 2026
16 of 19 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.

2 participants