Skip to content

Resolve remaining high/medium CodeQL alerts - #5629

Merged
norman-abramovitz merged 8 commits into
cloudfoundry:developfrom
nabramovitz:security/codeql-high-triage
Jul 11, 2026
Merged

Resolve remaining high/medium CodeQL alerts#5629
norman-abramovitz merged 8 commits into
cloudfoundry:developfrom
nabramovitz:security/codeql-high-triage

Conversation

@nabramovitz

Copy link
Copy Markdown
Contributor

Follow-up to #5624: clears the remaining 14 high / 6 medium CodeQL alerts on develop. 10 are fixed here; the other 10 were dismissed with per-alert justifications (test infrastructure, pass-through response writer, closed-union inputs, by-design per-endpoint TLS opt-in).

Backend:

  • Keep secrets out of logs and error messages: the invalid-provider error formatted the whole DatabaseConfig (including the DB password) into a message that reaches log.Fatal, and console setup dumped ConsoleConfig (including the client secret) at debug level (alerts 30, 31).
  • Kube terminal: verify the API server against the in-cluster CA from the service-account mount instead of unconditional InsecureSkipVerify; skipping remains only for the env-var-token dev setup with no CA file (alert 33).
  • Backup encryption: derive the AES-256 key with PBKDF2-HMAC-SHA256 (600k iterations, random salt behind a magic prefix) instead of a single unsalted SHA256, so a leaked backup file can't be brute-forced cheaply. Files without the prefix decrypt via the legacy path, so existing backups still restore; a round-trip test covers both formats (alert 34).
  • CF hosting: mark the JSESSIONID affinity cookie Secure - the plugin already rejects non-https X-Forwarded-Proto (alert 21).

Frontend / tools:

  • Entity catalog: constant first argument to console.warn/error so catalog-supplied types can't act as format strings (alerts 13, 14).
  • Remove the unused pathSet utility rather than guarding dead code (alert 15).
  • Guard chart-values merge and stb token-path projection against proto/constructor/prototype keys (alerts 16, 17).

Full make check gate is green; the backup change carries a new unit test for both the new and legacy formats.

Invalid-provider errors formatted the whole DatabaseConfig (including
the database password) into a message that reaches log.Fatal, and the
console setup dumped ConsoleConfig (including the client secret) at
debug level. Log only the provider name and UAA endpoint instead.

CodeQL: go/clear-text-logging (alerts 30, 31).
The terminal's exec websocket dialed the Kubernetes API server with
InsecureSkipVerify unconditionally. Read the in-cluster CA bundle from
the service-account mount (alongside the token we already read) and
verify against it; fall back to skipping verification only in the
env-var-token dev setup where no CA file is mounted.

CodeQL: go/disabled-certificate-check (alert 33).
Backup payload encryption keyed AES-256 with a single unsalted SHA256
of the user's password, allowing fast offline brute force of a leaked
backup file. Derive the key with PBKDF2-HMAC-SHA256 (600k iterations,
random 16-byte salt stored after a magic prefix). Files without the
prefix decrypt via the legacy path so existing backups still restore;
a round-trip test covers both formats.

CodeQL: go/weak-sensitive-data-hashing (alert 34).
The JSESSIONID cookie inherited session options without an explicit
Secure flag. CF hosting always terminates TLS (the plugin already
rejects non-https X-Forwarded-Proto), so mark the cookie Secure.

CodeQL: go/cookie-secure-not-set (alert 21).
Missing-entity warnings interpolated catalog-supplied endpoint/entity
types into the first console argument, which the console treats as a
format string. Move the dynamic detail to a separate argument.

CodeQL: js/tainted-format-string (alerts 13, 14).
pathSet had no callers left (pathGet remains in use) and its recursive
path assignment was flagged for prototype pollution. Delete it and its
spec instead of guarding dead code.

CodeQL: js/prototype-pollution-utility (alert 15).
doMergeObjects copies keys from parsed YAML/JSON into the target;
JSON.parse can produce an own __proto__ key, and assigning it walks
into Object.prototype. Skip __proto__/constructor/prototype keys.

CodeQL: js/prototype-pollution-utility (alert 16).
setPath assigned dot-separated path segments without filtering
__proto__/constructor/prototype; reject paths containing them.

CodeQL: js/prototype-pollution-utility (alert 17).
Comment thread src/jetstream/plugins/kubernetes/terminal/start.go Dismissed

@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 34536f6 into cloudfoundry:develop Jul 11, 2026
21 checks passed
@nabramovitz
nabramovitz deleted the security/codeql-high-triage branch July 11, 2026 17:09
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