Skip to content

Improve CodeQL compliance and OpenSSF Scorecard posture#14

Merged
jwilder merged 3 commits into
mainfrom
scorecard-improvements
Apr 9, 2026
Merged

Improve CodeQL compliance and OpenSSF Scorecard posture#14
jwilder merged 3 commits into
mainfrom
scorecard-improvements

Conversation

@jwilder
Copy link
Copy Markdown
Contributor

@jwilder jwilder commented Apr 9, 2026

Summary

Resolves the 7 pre-existing CodeQL alerts blocking PR merges and improves OpenSSF Scorecard compliance across several checks.

Changes

Phase 1: Fix CodeQL Alerts (unblocks all PR merges)

Alert Rule Resolution
#1 actions/missing-workflow-permissions Added permissions: contents: read to smoke-metalman.yaml
#2 py/clear-text-storage-sensitive-data Inline suppression — ephemeral bootstrap token in CI-only test code (e2e.py)
#3-5 go/insecure-hostkeycallback Inline suppressions — intentional design: SSH to newly provisioned machines with unknown host keys (machine_controller.go)
#6 go/zipslip Inline suppression — false positive: cleanedTarEntryName() already validates paths (download.go)
#7 go/disabled-certificate-check Inline suppression — intentional: InsecureSkipVerify used with VerifyConnection for BMC cert pinning (redfish/client.go)

Phase 2: CodeQL Workflow (Scorecard SAST check)

  • Added .github/workflows/codeql.yaml with Go, Python, and Actions analysis
  • Triggers on push/PR to main + weekly schedule
  • All actions SHA-pinned consistent with repo convention
  • Note: After merge, the GitHub "default setup" CodeQL should be disabled to avoid duplicate scans

Phase 3: Scorecard Quick Wins

  • Pinned-Dependencies: All 5 Containerfile base images pinned by digest (@sha256:...)
    • mcr.microsoft.com/azurelinux/base/core:3.0 (machina, metalman — builder + runtime stages)
    • docker.io/library/golang:1.26 (host-ubuntu2404)
    • docker.io/library/debian:bookworm-slim (host-ubuntu2404)
    • docker.io/library/ubuntu:noble (agent-ubuntu2404, agent-ubuntu2404-nvidia)
  • Code-Review: Added .github/CODEOWNERS assigning @Azure/unbounded-dev as default owners

Files Changed (12)

  • .github/CODEOWNERS — new
  • .github/workflows/codeql.yaml — new
  • .github/workflows/smoke-metalman.yaml — added permissions block
  • cmd/agent/internal/utilio/download.go — CodeQL suppression
  • cmd/machina/machina/controller/machine_controller.go — CodeQL suppressions (3 locations)
  • hack/agent/e2e-kind/e2e.py — CodeQL suppression
  • images/agent-ubuntu2404/Containerfile — digest pin
  • images/agent-ubuntu2404-nvidia/Containerfile — digest pin
  • images/host-ubuntu2404/Containerfile — digest pins (2 images)
  • images/machina/Containerfile — digest pins (2 stages)
  • images/metalman/Containerfile — digest pins (2 stages)
  • internal/metalman/redfish/client.go — CodeQL suppression

Verification

  • make lint — passes
  • make test — passes

- Add permissions: contents: read to smoke-metalman.yaml workflow
- Add CodeQL inline suppressions for 6 false-positive/intentional alerts
  (insecure-hostkeycallback, zipslip, disabled-certificate-check,
  clear-text-storage-sensitive-data)
- Create explicit CodeQL workflow (Go, Python, Actions) with SHA-pinned
  actions to satisfy OpenSSF Scorecard SAST check
- Pin all Containerfile base images by digest for Scorecard
  Pinned-Dependencies check
- Add .github/CODEOWNERS assigning @Azure/unbounded-dev as default owners
@jwilder jwilder requested a review from a team April 9, 2026 21:11
Comment thread hack/agent/e2e-kind/e2e.py Fixed
The repository already has CodeQL 'default setup' enabled in GitHub
settings, which scans Go, Python, and Actions. An explicit workflow
file cannot coexist with the default setup — GitHub rejects the SARIF
upload with: 'CodeQL analyses from advanced configurations cannot be
processed when the default setup is enabled.'

The default setup already satisfies the OpenSSF Scorecard SAST check.
Comment thread cmd/machina/machina/controller/machine_controller.go Fixed
Comment thread cmd/machina/machina/controller/machine_controller.go Fixed
Comment thread cmd/machina/machina/controller/machine_controller.go Fixed
Comment thread internal/metalman/redfish/client.go Fixed
CodeQL default setup does not honor inline comment-based suppressions.
The comments also caused CodeQL to see the alerts as 'new' on the PR
since the line content changed. The 7 pre-existing alerts have been
dismissed via the GitHub API instead.
@jwilder jwilder merged commit 6b71e37 into main Apr 9, 2026
12 of 13 checks passed
@jwilder jwilder deleted the scorecard-improvements branch April 9, 2026 22:04
plombardi89 added a commit that referenced this pull request May 14, 2026
GHAS / CodeQL flagged two "Incorrect conversion between integer
types" findings on the awss3 and azureblob origin adapters: the
List call sites cast a host-int maxResults to int32 without an
upper-bound check, so an untrusted caller passing
?max-keys=99999999999 would silently overflow MaxKeys/MaxResults
to a non-deterministic (often negative) int32 value before it
reached the SDK. The server-side parse (server.go:704-711) only
guards v > 0; the > int32 max case slipped through.

Per-adapter clamp instead of a server-side ceiling so each backend
keeps its own documented max:
- awss3.clampMaxKeys: 0..1000 (S3 ListObjectsV2 server-side cap)
- azureblob.clampMaxResults: 0..5000 (Azure ListBlobs server-side cap)

A server-wide cap would have artificially limited Azure callers to
the smaller of the two ceilings; clamping per-adapter keeps the
backends' native quotas reachable while making the int32 conversion
locally provable. Negative inputs collapse to 0 (which the
backends interpret as "use default") so the symmetric overflow
window is also closed.

Tests: TestClampMaxKeys and TestClampMaxResults table-drive every
boundary (zero, small positive, exact cap, cap+1, math.MaxInt32,
math.MaxInt, -1, math.MinInt). Both run as fast unit tests with
no SDK or network seam needed.

Resolves https://github.com/Azure/unbounded/security/code-scanning/14
Resolves https://github.com/Azure/unbounded/security/code-scanning/15
plombardi89 added a commit that referenced this pull request May 14, 2026
GHAS / CodeQL flagged two "Incorrect conversion between integer
types" findings on the awss3 and azureblob origin adapters: the
List call sites cast a host-int maxResults to int32 without an
upper-bound check, so an untrusted caller passing
?max-keys=99999999999 would silently overflow MaxKeys/MaxResults
to a non-deterministic (often negative) int32 value before it
reached the SDK. The server-side parse (server.go:704-711) only
guards v > 0; the > int32 max case slipped through.

Per-adapter clamp instead of a server-side ceiling so each backend
keeps its own documented max:
- awss3.clampMaxKeys: 0..1000 (S3 ListObjectsV2 server-side cap)
- azureblob.clampMaxResults: 0..5000 (Azure ListBlobs server-side cap)

A server-wide cap would have artificially limited Azure callers to
the smaller of the two ceilings; clamping per-adapter keeps the
backends' native quotas reachable while making the int32 conversion
locally provable. Negative inputs collapse to 0 (which the
backends interpret as "use default") so the symmetric overflow
window is also closed.

Tests: TestClampMaxKeys and TestClampMaxResults table-drive every
boundary (zero, small positive, exact cap, cap+1, math.MaxInt32,
math.MaxInt, -1, math.MinInt). Both run as fast unit tests with
no SDK or network seam needed.

Resolves https://github.com/Azure/unbounded/security/code-scanning/14
Resolves https://github.com/Azure/unbounded/security/code-scanning/15
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