Skip to content

Fix/mend sca 202609 - #343

Merged
hunterwei-ibm merged 8 commits into
masterfrom
fix/mend-sca-202609
Sep 25, 2026
Merged

hunterwei-ibm merged 8 commits into
masterfrom
fix/mend-sca-202609

Conversation

@hunterwei-ibm

@hunterwei-ibm hunterwei-ibm commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR bumps the metrics-agent to version 2.14.17 and includes a set of security hardening, code quality, and dependency update changes:

  • Path traversal protection: Introduces a new util.SafeJoin helper that validates joined file paths remain within the intended base directory, guarding against path traversal attacks. This is applied throughout kubernetes.go, util.go, and related files wherever paths were previously constructed via raw string concatenation (e.g., dir + "/" + file).
  • Dependency updates: Upgrades several dependencies to newer, patched versions:
    • github.com/aws/aws-sdk-go: v1.40.27 → v1.55.8
    • github.com/pelletier/go-toml/v2: v2.0.5 → v2.4.3
    • golang.org/x/net: v0.53.0 → v0.56.0
    • golang.org/x/sys: v0.45.0 → v0.46.0
    • golang.org/x/term: v0.43.0 → v0.44.0
    • Removes several now-unnecessary replace directives in go.mod (e.g., for golang.org/x/crypto, golang.org/x/net, google.golang.org/grpc, google.golang.org/protobuf, github.com/mattn/go-sqlite3) since the direct dependency versions now satisfy the CVE requirements.
  • Base Docker image update: Bumps Alpine from 3.24.1 → 3.24.2 and adds --no-cache to apk add commands to reduce image size and avoid stale cache issues.
  • Code quality / linter fixes:
    • Replaces unsafe type assertions (e.g., to.(*corev1.Pod)) with the two-value form (cast, _ := to.(*corev1.Pod)) throughout k8s_stats.go and raw_endpoint.go.
    • Uses typed switch variable (switch to := to.(type)) to eliminate redundant casts in sanitizeData and trimData.
    • Simplifies ObjectMeta.X field access to the promoted field shorthand (e.g., cast.ObjectMeta.ManagedFields → cast.ManagedFields).
    • Fixes ignored error return values (e.g., defer resp.Body.Close() → defer func() { _ = resp.Body.Close() }(), os.RemoveAll, os.Remove).
    • Simplifies HTTP status check condition from !(>= 200 && <= 299) to < 200 || > 299.
    • Replaces f.WriteString(fmt.Sprintf(...)) with fmt.Fprintf(f, ...).
    • Simplifies label access: n.ObjectMeta.Labels → n.Labels.
    • Fixes type assertion syntax on shared informers in tests.
    • Removes a duplicate _ "strconv" import.
  • golangci-lint version bump: Bumps golangci-lint version from v1.64.2 (build by Go 1.24) to v2.7.2 due to v1.64.2 incompatibility to Go 1.25.
  • Misc. code linting: Misc. code linting fixes according to golangci-lint v2.7.2 run.

Where should the reviewer start?

  1. util/util.go — Review the new SafeJoin function, which is the core security change in this PR.
  2. kubernetes/kubernetes.go — Review all call sites where SafeJoin replaces raw path concatenation (createMSD, fetchNodeBaselines, updateNodeBaselines, createAgentStatusMetric, fetchDiagnostics).
  3. go.mod — Review the dependency upgrades and removed replace directives to confirm no regressions are introduced.
  4. retrieval/k8s/k8s_stats.go and retrieval/raw/raw_endpoint.go — Review the type assertion and ObjectMeta simplifications for correctness.

How should this be manually tested?

  1. Build and run locally:
    go build ./...
    go test ./...
  2. Docker image build:
    docker build -f deploy/docker/Dockerfile .
    Confirm the image builds successfully on Alpine 3.24.2.
  3. Helm chart deployment:
    • Deploy the updated Helm chart (version: 2.14.16) to a test Kubernetes cluster and confirm the agent pod starts, collects metrics, and exports data without errors.
  4. Path traversal validation:
    • Review/run the new TestSafeJoin unit tests to confirm traversal attempts (e.g., ../../etc/passwd) are correctly rejected.
  5. End-to-end metric collection:
    • Verify metrics-agent raw data is uploaded to S3 bucket for the test K8s cluster

Any background context you want to provide?

  • Several replace directives in go.mod were previously needed to pin vulnerable transitive dependency versions (e.g., golang.org/x/net for CVE-2026-39821). The direct dependency upgrades in this PR bring in versions that already satisfy those requirements, so the overrides are no longer needed and have been removed to reduce maintenance overhead.
  • The SafeJoin utility is being introduced proactively to harden all file path construction in the agent against potential path traversal, which is a common vulnerability class in agents that write files based on cluster-derived data (e.g., node names, timestamps).
  • The --no-cache flag added to apk add in the Dockerfile is a Docker best practice that avoids caching package index files in the image layer, reducing the final image size.

What picture best describes this PR (optional but encouraged)?

security hardening + dependency updates keeping the house in order.

What are the relevant Github Issues?

No specific GitHub issues were referenced in the diff. Link any relevant CVE tracking issues or dependency audit tickets here.

Developer Done List

  • Tests Added/Updated
    • New TestSafeJoin tests in util/util_test.go
    • New TestCreateMSD, TestFetchNodeBaselines, TestUpdateNodeBaselines tests in kubernetes/kubernetes_test.go
    • Updated existing tests for linter compliance
  • Updated README.md
  • Verified backward compatible — all changes are internal implementation details; no public API or configuration changes
  • Verified database migrations will not be catastrophic — N/A (no database)
  • Considered Security, Availability and Confidentiality — SafeJoin directly addresses path traversal; dependency upgrades address known CVEs

For the Reviewer:

  • Pay particular attention to the SafeJoin implementation and its call sites to ensure no valid path construction is accidentally rejected (e.g., paths derived from timestamps or UIDs).
  • Confirm that removing the go.mod replace directives for golang.org/x/net, google.golang.org/grpc, etc. does not reintroduce vulnerable transitive versions — run go mod graph or govulncheck if in doubt.
  • The two-value type assertion form (cast, _ := to.(Type)) silently ignores assertion failures. Verify this is safe given the call sites always pass the correct concrete types.

By approving this PR, the reviewer acknowledges that they have checked all items in this done list.

Reviewer/Approval Done List

  • Tests Pass Locally
  • CI Build Passes
  • Verified README.md is updated
  • Verified changes are backward compatible
  • Reviewed impact to Security, Availability and Confidentiality (if issue found, add comments and request changes)

hunterwei-ibm and others added 4 commits September 21, 2026 16:39
* fix(CLDYCON-7678): guard against path traversal in metric sample dir to fix Mend SAST vulnerability CWE-22 Path/Directory Traversal.

* fix(CLDYCON-7678): expand path traversal guards in kubernetes to remediate CWE-73 File Manipulation vulnerability.

* Address PR comments: fix safeJoin to allow base-equal paths and update linter config; update correspondant tests.

* refactor(CLDYCON-7678): simplify type assertions and fix linter warnings

* refactor(CLDYCON-7678): move safeJoin to util and simplify assertions

* chore(CLDYCON-7678): migrate golangci-lint config to v2 format and bump to v2.7.2 to match other repos of Containers team.
@CLAassistant

CLAassistant commented Sep 23, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@troy-chuang troy-chuang 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

@hunterwei-ibm
hunterwei-ibm merged commit 93667cd into master Sep 25, 2026
3 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.

4 participants