security: pin the gitleaks download with a SHA-256 checksum#483
Merged
Conversation
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
This was referenced May 30, 2026
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feaed83814
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Go toolchain/dependencies to address reported CVEs and strengthens the CI security workflow by making security scanners more tamper-resistant and stricter.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Add
toolchain go1.26.3and bumpgolang.org/x/net(and related indirect sums) to newer patched versions. - Harden gitleaks installation in CI by verifying the downloaded release tarball against a pinned SHA-256.
- Tighten the standalone gosec CI job by removing
-no-failand excluding the existing accepted baseline rule set.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
go.mod |
Pins the Go toolchain and bumps indirect x/net / x/term versions. |
go.sum |
Updates module checksums to match the bumped indirect dependencies. |
.github/workflows/security.yml |
Hardens gitleaks install via checksum verification and turns gosec into a failing gate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeremy
force-pushed
the
security/05-perms-and-exec-args
branch
from
e4a7a62 to
a3ff017
Compare
jeremy
force-pushed
the
security/06-supply-chain-ci
branch
from
feaed83 to
3d62a58
Compare
jeremy
force-pushed
the
security/05-perms-and-exec-args
branch
from
a3ff017 to
c1d95a4
Compare
jeremy
force-pushed
the
security/06-supply-chain-ci
branch
from
3d62a58 to
54c2ef1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54c2ef1b25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jeremy
force-pushed
the
security/05-perms-and-exec-args
branch
from
c1d95a4 to
fc26287
Compare
jeremy
force-pushed
the
security/06-supply-chain-ci
branch
from
54c2ef1 to
6f3eab7
Compare
jeremy
force-pushed
the
security/05-perms-and-exec-args
branch
from
fc26287 to
7ae26db
Compare
jeremy
force-pushed
the
security/06-supply-chain-ci
branch
from
6f3eab7 to
41c42c5
Compare
jeremy
force-pushed
the
security/05-perms-and-exec-args
branch
from
7ae26db to
36ba2ba
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
parsePath now validates absolute URLs against the configured base URL host:
a path is accepted only if it's relative or an absolute Basecamp URL on the
same host (path extracted, account segment dropped — the SDK re-prefixes the
configured account). Absolute URLs on any other host are rejected before any
network call, so the SDK never attaches the Authorization: Bearer header to a
foreign host. A stray leading slash ('/https://evil/…') and mixed-case schemes
('HTTPS://…') are normalized first so neither can smuggle an absolute URL past
the host check. Error propagated through get/post/put/delete; api_test.go
covers same-host extraction, query preservation, and foreign-host rejection.
API-controlled strings (names, summaries, notices, headlines, affordance
commands) reached styled/markdown sinks and the --watch TUI without ANSI
stripping, allowing terminal/OSC injection. Sanitize WithSummary/WithNotice/
WithDiagnostic at the source, the timeline watch TUI fields (creator name
stripped before the empty check so an all-escape name still falls back to a
placeholder), and presenter format helpers, with defense-in-depth at the
render sinks. Stripping is centralized in RenderTemplate + RenderHeadline so
schema headlines ({{.name}}/{{.subject}}/{{.content}}) and affordance commands
are all covered, including the identity-label headline fallback.
Assert the exact sanitized string instead of mirroring ansi.Strip, which is a weaker contract than richtext.SanitizeTerminal and ties the expectation to the third-party stripper's behavior.
Stripping keys let a hostile key like "\x1b[31mtitle" collapse onto a legitimate "title", silently replacing or hiding that field in TTY --jq output. Keys changed by sanitization are now visibly escaped via strconv.Quote, so clean keys always keep their slot and no entry is dropped. Also update a stale test comment that referenced ansi.Strip.
The escaped form of a hostile key could still collide with a literal key that mimics escape notation (e.g. a real ESC-prefixed title vs the text "\x1b[31mtitle"), nondeterministically dropping one entry. Clean keys now claim their names first, and escaped keys re-quote with delimiters until unique, in sorted order for deterministic output.
Request IDs and string ids print to line-oriented sinks, but SanitizeTerminal preserves newlines/tabs (which inject lines or columns into ids output) and deletes bare CR (which joins adjacent words). A shared sanitizeSingleLine helper normalizes CR/CRLF to newlines first, then strips and collapses all whitespace to single spaces.
- Scheme-validate the discovery authorization_endpoint (https, or http on loopback) before it's dispatched to the OS browser launcher, so a hostile discovery doc can't hand the OS a file:// (or other) URL. - Enforce RequireSecureURL on the DCR registration_endpoint. - Set CheckRedirect on the OAuth HTTP client so token exchange/refresh POST bodies (auth code, refresh_token) aren't replayed to a redirect target. Idempotent GET/HEAD (OAuth discovery) may still follow redirects, so we don't break discovery or force an unintended Launchpad fallback.
- Trust-gate cache_dir, cache_enabled, llm_model, llm_max_concurrent and llm_token_budget so an untrusted local/repo config can't redirect cache writes or amplify paid-LLM usage; only honored from trusted sources. 'config set' now also warns when these keys are written to an untrusted local config so the value isn't silently ignored on the next load. - Scheme/host-validate llm_endpoint on accept (rejects file://, hostless and malformed forms) and re-validate at the root.go enforcement point so env/ profile-sourced endpoints can't slip a non-http(s) scheme past RequireSecureURL, which only blocks http:// for non-localhost; this prevents leaking llm_api_key in cleartext. - Gate the completion profile loader behind the TrustStore and strip control characters from completion descriptions.
- Create the global config dir at 0700 (it can hold credentials.json) in the
skill-refresh and update-notice bootstrap paths, and harden it once early in
root.go so an existing 0755 dir from an older version is tightened. The
hardening is an atomic fd-based open(O_NOFOLLOW) -> fstat -> fchmod sequence
that verifies a self-owned real directory on the open fd, so symlink swaps
and writable ancestors can't redirect the chmod (no Lstat->Chmod TOCTOU),
and a config dir under a sticky world-writable parent like /tmp is hardened
like any other.
- Whitelist the plugin --scope value {user,project,local,global} read from
installed_plugins.json before passing it to 'claude plugin uninstall',
preventing a '-'-leading value from injecting a flag into the argv.
Verify the gitleaks release tarball against a pinned SHA-256 before installing it in the secret-scanning workflow, so a compromised or tampered release asset cannot execute in CI. (Originally this PR also set toolchain go1.26.3 and bumped golang.org/x/net to v0.55.0 to clear govulncheck-reachable advisories; main has since moved to go 1.26.5 with those same dependency versions, so after rebasing the dependency changes are subsumed and only the gitleaks pin remains.) The standalone gosec scan stays advisory (-no-fail, full SARIF upload). Its enforcing counterpart is golangci-lint (make lint), which honors the repo's per-site //nolint:gosec suppressions; the standalone binary can't read those, and globally excluding rule IDs (e.g. G101/G204) to make it hard-fail would blind release scans to new hard-coded creds / unsafe exec.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if err := syscall.Fstat(targetFd, &st); err != nil { | ||
| return | ||
| } | ||
| if st.Mode&syscall.S_IFMT != syscall.S_IFDIR || st.Uid != uint32(euid) { //nolint:gosec // G115: Geteuid() returns a valid non-negative uid that fits in uint32 |
| if st.Mode&0o022 != 0 && st.Mode&unix.S_ISVTX == 0 { | ||
| return false | ||
| } | ||
| if st.Uid != uint32(euid) && st.Uid != 0 { //nolint:gosec // G115: Geteuid() returns a valid non-negative uid that fits in uint32 |
| if !ok { | ||
| return false | ||
| } | ||
| return st.Uid == uint32(uid) //nolint:gosec // G115: Geteuid() returns a valid non-negative uid that fits in uint32 |
| func uninstallUnscoped(ctx context.Context, claudePath, key string) bool { | ||
| n := 0 | ||
| for i := 0; i < 10; i++ { | ||
| c := exec.CommandContext(ctx, claudePath, "plugin", "uninstall", key) //nolint:gosec // G204: claudePath from FindClaudeBinary |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pin the gitleaks download with a SHA-256 checksum
(Threat: T5 supply chain — an unpinned release download means a compromised or tampered gitleaks release asset executes in CI.)
gitleaks pinning: the secret-scanning workflow now verifies the gitleaks release tarball against a pinned SHA-256 (from the release
checksums.txt) before installing it:gosec gate (unchanged, rationale documented): the standalone gosec scan stays advisory (
-no-fail, with full SARIF upload to the Security tab). Its enforcing counterpart isgolangci-lint(make lint), which honors the repo's per-site//nolint:gosecsuppressions; the standalone binary can't read those, and globally excluding rule IDs (e.g. G101/G204) to make it hard-fail would blind release scans to new hard-coded creds / unsafe exec.History
This PR originally also set
toolchain go1.26.3and bumpedgolang.org/x/netto v0.55.0 /golang.org/x/termto v0.43.0, takinggovulncheck ./...from 6 reachable advisories to "No vulnerabilities found." Main has since independently moved togo 1.26.5with those same dependency versions, so after rebasing onto current main the dependency changes are subsumed and only the gitleaks pin remains.Part 6/6 of the stacked security series. Base:
security/05-perms-and-exec-args.📚 Stack (merge bottom-up)
apito prevent token leak #478 — reject foreign-host URLs inapi(basemain)Each is independent except #482 depends on #481 (shared
root.go). #478 can land first/alone.Summary by cubic
Pins and verifies the
gitleaksv8.21.2 Linux tarball in CI using a release SHA-256 to block tampered downloads; extracts only the binary and installs it withinstall -m 0755to/usr/local/bin(usingGITLEAKS_VERSION/GITLEAKS_SHA256). Thegosecscan stays advisory (-no-fail) with SARIF upload; enforcement remains viagolangci-lint.Written for commit 1405a57. Summary will update on new commits.