Skip to content

fix(jetstream): SSRF + path-traversal guards, patched build-chain deps - #5624

Merged
norman-abramovitz merged 9 commits into
cloudfoundry:developfrom
nabramovitz:security-devkit-overrides
Jul 11, 2026
Merged

norman-abramovitz merged 9 commits into
cloudfoundry:developfrom
nabramovitz:security-devkit-overrides

Conversation

@nabramovitz

@nabramovitz nabramovitz commented Jul 11, 2026 •

Copy link
Copy Markdown
Contributor

Batches two security-findings fixes surfaced by the scanner baseline.

1. SSRF guard on operator-supplied fetch URLs

Two backend endpoints fetch a URL taken from the request using the
standard http client:

  • GET /pp/v1/kube/cert (kubernetes/main.go RequiresCert) — probes a
    Kube API URL to see whether it needs a client cert.
  • Helm chart download (kubernetes/install_release.go loadChart).

Both are admin-gated, but a Stratos admin is not necessarily an infra
admin: pointing either at http://169.254.169.254/ or a loopback
service lets the backend read the host's cloud metadata / IAM
credentials — an SSRF escalation.

Fix adds GetGuardedHttpClient, a client whose dialer rejects loopback
and link-local addresses through a net.Dialer.Control hook. The check
runs on the resolved IP at dial time, so DNS rebinding cannot slip a
blocked address past it. Private RFC1918 ranges stay allowed because
Kubernetes API servers legitimately live there. Both sinks route through
it and validate/normalise the URL first (require an http(s) scheme and
host; join the path instead of string concatenation).

Closes the two go/request-forgery CodeQL alerts. A third
(authuaa.go) is admin-configured endpoint metadata rather than request
input and is left for a separate dismissal.

2. Patched build-chain deps via devkit overrides

Force patched transitive versions through devkit's npm overrides to
clear the frontend Dependabot alerts against its package-lock:

Package Before After Line
undici 7.24.4 7.28.0 —
vite 7.3.2 7.3.5 —
webpack-dev-server 5.2.3 5.2.5 —
piscina 5.1.4 5.2.0 —
@babel/core 7.29.0 7.29.6 —
http-proxy-middleware 3.0.5 3.0.7 v3 (CRLF-injection fix)
http-proxy-middleware 2.0.9 2.0.10 v2, scoped under webpack-dev-server

All are dev-server / build-chain tooling that never ships to runtime.
esbuild and uuid are left unchanged: esbuild's fix (0.28.x) is outside
vite's esbuild@^0.27.0 range, and uuid's fix (11.x) is a major jump
sockjs (peer ^8.3.2) will not accept. Both clear once those
upstreams widen.

3. Kubernetes plugin build fix

The plugin module pinned spf13/cobra to v0.0.3 via a replace directive
from the 2019 Helm 3 migration, which predates the cobra API that
k8s.io/kubectl v0.35.1 now requires — so the module no longer compiled.
Removed the stale replace so cobra resolves to the v1.10.2 already
required by the graph. Our code does not use cobra directly (it enters
only through helm/kubectl internals). This also lets the SSRF plugin
changes above actually compile.

4. cfapppush path traversal in folder/file upload

The deploy websocket's folder/file source handler joined client-supplied
folder names and file paths straight onto the temp upload dir, so a
message like ../../../home/vcap/app/x escaped the sandbox — os.Mkdir
created dirs and os.WriteFile wrote attacker-controlled content outside
tempDir, as the backend (vcap) user, before any Cloud Foundry
authorization check. Reachable by any authenticated Stratos user.

Added safeUploadJoin, which rejects non-local names (filepath.IsLocal)
before joining — the same guard #5620 applied to archive extraction in
unarchive.go, which never covered this separate upload loop. Both the
folder-create and file-write sinks route through it; legitimate nested
paths still pass. Unit test covers traversal rejection and nested-path
acceptance.

Closes the two go/path-injection CodeQL alerts in deploy.go (297,
338). The remaining path-injection alerts (monocular, analysis/container)
are triaged separately: most are not exploitable, and the few real ones
are lower-urgency (a hostile registered Helm repo; a tech-preview-gated
sidecar).

5. monocular chart-cache path traversal

The chart cache folder joined chart Name, Version and EndpointID
straight onto the cache dir. Name/Version come from an external Helm
repository's index.yaml, so a hostile registered repo could set a name
like ../../.. and make the sync worker write the digest, create
directories, save the downloaded icon, and extract archive files outside
the cache — as the backend user. Add safeSegment and apply it to all
three components at the single choke point getChartCacheFolder, so
every cache sink is confined. Legitimate names/versions/GUIDs are
unchanged. Closes the four go/path-injection alerts in cache.go (119,
168, 260, 312).

6. analysis container report-path traversal

The analysis container service built filesystem paths from its HTTP route
parameters (user/endpoint/id/file) and the X-Stratos-Analaysis-ID
header without traversal checks, so a ../.. value escaped reportsDir:
report served an arbitrary *.json, delete/deleteEndpoint ran
os.RemoveAll on an arbitrary directory, and the run handler wrote report
files outside the tree. The container has no auth of its own (it is
intended as a tech-preview-gated sidecar), so this is reachable by anyone
who can reach its port. Add validateSegment for the single-segment route
params and reject a non-local ID header with filepath.IsLocal (the
header seeds job.Folder, which the kubescore/popeye analyzers write
into). Mirrors the existing validateContentID/validateNamespace
guards. Closes the go/path-injection alerts in routes.go (39, 50, 71),
run.go (44), kubescore.go (58, 62) and popeye.go (73, 77).

7. Code Quality (CodeQL correctness) alerts

Clears the remaining CodeQL Code Quality list beyond the incomplete-sanitization fixes above:

  • js/incomplete-url-substring-sanitization (kube-config-auth.helper.ts 71, 112) — provider detection used server.indexOf('azmk8s.io'|'eks.amazonaws.com'), which matches the domain anywhere in the URL. Replaced with serverHostEndsWith, a hostname-suffix check. Only the provider label is affected, but the match is now correct.
  • js/shell-command-injection-from-environment (build-orchestrator.js 130, post-setup.cjs 24) — two build helpers interpolated paths into shell strings for execSync. Switched to execFileSync with an argv array (plus fs.renameSync for the .js/.cjs shuffle), removing the shell. Build-only tooling with trusted input, but it removes the injection-shaped pattern.

With the incomplete-sanitization fixes, all 7 correctness-tagged Code Quality alerts are addressed.

Verification

  • cfapppush + core jetstream modules build; guardedDialControl and safeUploadJoin unit tests pass
    (blocks loopback / link-local / metadata / unparseable, allows public
    and RFC1918).
  • bun run build-devkit builds clean; npm audit on devkit drops to the
    two upstream-blocked advisories above.

Notes for reviewers

  • Dependabot scans devkit's package-lock.json but not the root
    bun.lock, so the same patched-vs-vulnerable versions also exist,
    unreported, in the root tree. Folding devkit into the root bun
    workspace (possible once it moves off Angular 21 onto 22 like the rest
    of the repo) is the follow-up that removes the duplicate npm island.
  • Pre-existing lint (not fixed here, does not block CI): go test ./...
    on the kubernetes plugin module reports several go vet
    non-constant-format-string warnings in helm_client.go,
    kube_dashboard.go and install_release.go:113. CI compiles the
    plugin via extra_plugins.go rather than running its module tests, so
    these do not gate — worth a separate cleanup.

Force patched transitive versions through devkit's npm overrides to
clear the frontend Dependabot alerts against its package-lock:

  undici              7.24.4 -> 7.28.0
  vite                7.3.2  -> 7.3.5
  webpack-dev-server  5.2.3  -> 5.2.5
  piscina             5.1.4  -> 5.2.0
  @babel/core         7.29.0 -> 7.29.6
  http-proxy-middleware  3.0.5 -> 3.0.7 (v3 line, CRLF-injection fix)
                         2.0.9 -> 2.0.10 (v2 line, scoped under
                         webpack-dev-server)

All are dev-server / build-chain tooling that never ships to the
runtime. Each patched version is within the consumers' declared peer
ranges; the exact pins they replace come from @angular-devkit/build-angular
and @angular/build.

esbuild and uuid are left unchanged: esbuild's fix (0.28.x) is outside
vite's esbuild@^0.27.0 range, and uuid's fix (11.x) is a major jump
sockjs (peer ^8.3.2) will not accept. Both clear once those upstreams
widen.
@nabramovitz
nabramovitz marked this pull request as draft July 11, 2026 08:25
The Kube API cert probe (/kube/cert) and the Helm chart download both
fetch a URL taken from the request, using the standard http client, so
an authenticated admin could point them at loopback or the cloud
metadata endpoint (169.254.169.254) and read the backend host's
credentials.

Add GetGuardedHttpClient: a client whose dialer rejects loopback and
link-local addresses via a net.Dialer.Control hook. The check runs on
the resolved IP at dial time, so DNS rebinding cannot slip a blocked
address past it. Private RFC1918 ranges stay allowed because Kubernetes
API servers legitimately live there.

Route both sinks through it, and validate/normalise the URL first
(require an http(s) scheme and host, join the path rather than
concatenating strings).

CodeQL: go/request-forgery in kubernetes/main.go and
kubernetes/install_release.go.
@nabramovitz nabramovitz changed the title chore(deps): pin patched build-chain deps via devkit overrides fix(jetstream): SSRF guard + patched build-chain deps Jul 11, 2026
Comment thread src/jetstream/plugins/kubernetes/install_release.go Dismissed
Comment thread src/jetstream/plugins/kubernetes/main.go Dismissed
The plugin module pinned github.com/spf13/cobra to v0.0.3 via a replace
directive dating from the 2019 Helm 3 migration. That version predates
the cobra API (ShellCompDirective, SetOut/SetErr,
RegisterFlagCompletionFunc) that k8s.io/kubectl v0.35.1 now requires, so
the module no longer compiled.

Remove the replace so cobra resolves to the v1.10.2 already required by
the graph, and tidy in the matching go.sum entries. The module builds
again; our code does not use cobra directly (it enters only through
helm/kubectl internals).
The deploy websocket's folder/file source handler joined client-supplied
folder names and file paths straight onto the temp upload dir, so a
message like "../../../home/vcap/app/x" escaped the sandbox: os.Mkdir
created directories and os.WriteFile wrote attacker-controlled content
outside tempDir, as the backend (vcap) user, before any Cloud Foundry
authorization check. Any authenticated Stratos user could reach it.

Add safeUploadJoin, which rejects names that are not local
(filepath.IsLocal) before joining — the same guard cloudfoundry#5620 applied to
archive extraction in unarchive.go, which never covered this separate
upload loop. Route both the folder-create and file-write sinks through
it. Legitimate nested paths still pass; only traversal and absolute
paths are rejected.

CodeQL: go/path-injection in cfapppush/deploy.go (297, 338).
@norman-abramovitz norman-abramovitz changed the title fix(jetstream): SSRF guard + patched build-chain deps fix(jetstream): SSRF + path-traversal guards, patched build-chain deps Jul 11, 2026
The chart cache folder was built by joining chart Name, Version and
EndpointID straight onto the cache dir. Name and Version come from an
external Helm repository's index.yaml, so a hostile (or compromised)
registered repository could set a name like "../../.." and traverse out
of the cache directory: the sync worker then wrote the digest file,
created directories, saved the downloaded icon, and extracted archive
files outside the cache, as the backend user.

Add safeSegment and apply it to all three components at the single choke
point getChartCacheFolder, so every cache sink (digest write, MkdirAll,
icon download, archive extract) is confined. Legitimate chart
names/versions and endpoint GUIDs are unchanged.

CodeQL: go/path-injection in monocular/cache.go (119, 168, 260, 312).
The analysis container service built filesystem paths from its HTTP route
parameters (user/endpoint/id/file) and the X-Stratos-Analaysis-ID header
without traversal checks. A "../.." value escaped reportsDir: report
served an arbitrary *.json (routes.go), delete/deleteEndpoint ran
os.RemoveAll on an arbitrary directory, and the run handler created and
wrote report files outside the tree. The container has no auth of its
own, so this is reachable by anyone who can reach its port.

Add validateSegment for the single-segment route parameters, and reject a
non-local ID header with filepath.IsLocal (the header is the nested
"user/endpoint/id" path, and also seeds job.Folder that the kubescore and
popeye analyzers write into). Mirrors the existing validateContentID /
validateNamespace boundary guards.

CodeQL: go/path-injection in routes.go (39, 50, 71), run.go (44),
kubescore.go (58, 62), popeye.go (73, 77).
Two String.replace calls used a string argument, which replaces only the
first match. prepareRegexPattern converted only the first '*'/'**' in a
.cfignore glob, so a pattern like "src/*/x/*.js" left later wildcards as
literal asterisks; and the kube-dashboard hash handler decoded only the
first %3F/%3D, mangling URLs with more than one query parameter. Switch
both to global regexes so every occurrence is transformed.

CodeQL: js/incomplete-sanitization in deploy-application-ignorefiles.ts
(101) and kubernetes-dashboard.component.ts (170, 171).
parseAuth guessed the cluster provider (AKS/EKS) with
server.indexOf('azmk8s.io')/'eks.amazonaws.com', which matches the
domain anywhere in the URL — https://evil.com/azmk8s.io or
https://azmk8s.io.evil.com would both be mislabelled. Add
serverHostEndsWith, which parses the URL and matches the hostname
suffix. Only the provider label/subtype is affected, but the check is
now correct.

CodeQL: js/incomplete-url-substring-sanitization in
kube-config-auth.helper.ts (71, 112).
Two build helpers built shell command strings with interpolated paths
and passed them to execSync. Switch both to execFileSync with an argv
array (and fs.renameSync for the .js<->.cjs shuffle in post-setup), so
paths are never parsed by a shell. Build-only tooling with trusted
input, but it removes the injection-shaped pattern; post-setup now also
restores the original filename even when the script fails.

CodeQL: js/shell-command-injection-from-environment in
build-orchestrator.js (130) and post-setup.cjs (24).
@norman-abramovitz
norman-abramovitz marked this pull request as ready for review July 11, 2026 13:26

@norman-abramovitz norman-abramovitz 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.

lgtm

@norman-abramovitz
norman-abramovitz merged commit 754fa7b into cloudfoundry:develop Jul 11, 2026
21 checks passed
@nabramovitz
nabramovitz deleted the security-devkit-overrides branch July 11, 2026 15:20
nabramovitz added a commit to nabramovitz/stratos that referenced this pull request Aug 26, 2026
The ArtifactHub cache paths were built by joining route parameters
straight onto the cache directory in four places. This is not a live
traversal — a path parameter cannot hold a separator (the router does
not unescape path parameter values, and an encoded one decodes before
matching and simply fails to match the route), and repo, name and
version are underscore-joined into a single segment. The alerts over
this code are dismissed on develop for those reasons.

It is still worth not depending on that. The reasoning rests on
framework behaviour rather than on anything this code does, the
endpoint component is joined as a segment of its own, and the sibling
repository-sync cache has been confined at a choke point since cloudfoundry#5624.
ahCacheFolder and ahChartCacheFolder put every component through the
same safeSegment, so the property holds structurally instead of by
argument. Legitimate GUIDs, repository names and versions are
unchanged, so no existing cache entry is orphaned — there is a test
pinning both halves of that.

This does not clear the CodeQL alerts: the query does not model
safeSegment, which is why the equivalent cache.go findings are
dismissed rather than fixed.
norman-abramovitz pushed a commit that referenced this pull request Aug 27, 2026
The ArtifactHub cache paths were built by joining route parameters
straight onto the cache directory in four places. This is not a live
traversal — a path parameter cannot hold a separator (the router does
not unescape path parameter values, and an encoded one decodes before
matching and simply fails to match the route), and repo, name and
version are underscore-joined into a single segment. The alerts over
this code are dismissed on develop for those reasons.

It is still worth not depending on that. The reasoning rests on
framework behaviour rather than on anything this code does, the
endpoint component is joined as a segment of its own, and the sibling
repository-sync cache has been confined at a choke point since #5624.
ahCacheFolder and ahChartCacheFolder put every component through the
same safeSegment, so the property holds structurally instead of by
argument. Legitimate GUIDs, repository names and versions are
unchanged, so no existing cache entry is orphaned — there is a test
pinning both halves of that.

This does not clear the CodeQL alerts: the query does not model
safeSegment, which is why the equivalent cache.go findings are
dismissed rather than fixed.
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