Skip to content

chore: resolve open CodeQL security alerts#13

Merged
msukkari merged 1 commit intomainfrom
msukkari/codeql-fixes
Apr 21, 2026
Merged

chore: resolve open CodeQL security alerts#13
msukkari merged 1 commit intomainfrom
msukkari/codeql-fixes

Conversation

@msukkari
Copy link
Copy Markdown

Summary

Resolves the 12 open CodeQL alerts on sourcebot-dev/zoekt.

Open Dependabot alerts (informational)

Alerts #11 (grpc critical), sourcegraph#14 and sourcegraph#15 (otel medium/high) should be automatically closed on the next Dependabot rescan — they were fixed by the dependency bumps in #12 (already merged to main). This PR does not need to touch Dependabot.

CodeQL fixes in this PR

Alert Rule Severity Fix
#12 go/clear-text-logging high Redact URL userinfo before logging git clone args in gitindex/clone.go. Exec args unchanged.
#13 go/incorrect-integer-conversion high api.go: replace ParseInt(_, 10, 64) + int() with strconv.Atoi for tenantID.
sourcegraph#17 go/incorrect-integer-conversion high sg.go: replace Atoi + uint32() with ParseUint(_, 10, 32) for SG_ID.
#2-8, sourcegraph#23 actions/missing-workflow-permissions medium Add top-level permissions: contents: read to ci.yml and buf-breaking-check.yml (least-privilege default for GITHUB_TOKEN).
#1 actions/untrusted-checkout/high high semgrep.yml: switch trigger from pull_request_target to pull_request so PR code is only ever checked out without write scopes / access to secrets like GH_SEMGREP_SAST_TOKEN.

Test plan

  • go build ./... passes.
  • go test -count=1 -short ./gitindex/... ./cmd/zoekt-sourcegraph-indexserver/... passes.
  • Verified redactURLCredentials behavior: https://user:pass@github.com/foo/bar.githttps://redacted@github.com/foo/bar.git; non-URL arguments (--verbose, /tmp/foo.git, git@github.com:foo/bar.git) pass through unchanged.

CodeQL alerts addressed:

- gitindex/clone.go:60 (high, go/clear-text-logging): the clone command
  was logged with its full args, which may include URLs containing basic
  auth credentials. Added a URL-aware redactor that strips userinfo
  before logging. Actual exec args are unchanged.

- api.go:668 (high, go/incorrect-integer-conversion): replaced
  ParseInt(_, 10, 64) + int(id) with strconv.Atoi for the tenantID
  RawConfig value so no truncating int64 -> int conversion occurs.

- cmd/zoekt-sourcegraph-indexserver/sg.go:608 (high,
  go/incorrect-integer-conversion): replaced Atoi + uint32(id) with
  ParseUint(_, 10, 32) so the value is bounded to uint32 at parse time.

- .github/workflows/ci.yml and buf-breaking-check.yml (medium x8,
  actions/missing-workflow-permissions): added top-level
  `permissions: contents: read` to follow least-privilege for
  GITHUB_TOKEN.

- .github/workflows/semgrep.yml (high,
  actions/untrusted-checkout/high): the scan ran on pull_request_target
  while checking out the PR head, giving PR code access to secrets
  (GH_SEMGREP_SAST_TOKEN, security-events:write). Switched the trigger
  to pull_request so the PR head is only ever checked out in an
  untrusted context with no access to write scopes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@msukkari msukkari merged commit 7c6c629 into main Apr 21, 2026
14 of 16 checks passed
msukkari added a commit to sourcebot-dev/sourcebot that referenced this pull request Apr 21, 2026
sourcebot-dev/zoekt#13 merged as 7c6c629f. Updating the submodule
pointer from the feature-branch tip (945c3e96) to the merge commit
on main so vendor/zoekt tracks canonical history before merging.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
msukkari added a commit to sourcebot-dev/sourcebot that referenced this pull request Apr 21, 2026
* chore: bump vendor/zoekt to include CodeQL security fixes

Pulls in sourcebot-dev/zoekt#13 (open), which resolves all open
CodeQL security alerts on the zoekt repo:

- go/clear-text-logging (high) in gitindex/clone.go
- go/incorrect-integer-conversion (high) in api.go and
  zoekt-sourcegraph-indexserver/sg.go
- actions/missing-workflow-permissions (medium x8) in ci.yml and
  buf-breaking-check.yml
- actions/untrusted-checkout/high (high) in semgrep.yml

Also carries through the dependency bumps from sourcebot-dev/zoekt#11
and #12 (go-git 5.18.0, grpc 1.80.0, otel 1.43.0) that were merged
after #1140 so weren't included when main shipped the original zoekt
sync.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: repoint vendor/zoekt at sourcebot-dev/zoekt@main merge commit

sourcebot-dev/zoekt#13 merged as 7c6c629f. Updating the submodule
pointer from the feature-branch tip (945c3e96) to the merge commit
on main so vendor/zoekt tracks canonical history before merging.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: repoint vendor/zoekt at upstream-ancestry merge commit

sourcebot-dev/zoekt#10 was squash-merged into zoekt@main, which
flattened the merge commit and left GitHub reporting the fork as 108
commits behind sourcegraph/zoekt:main even though all upstream content
was present. Fixed by performing a 'git merge -s ours upstream/main'
on zoekt@main: this records upstream/main as a second parent without
changing any files, restoring the ancestry link.

Bumping this submodule pointer from 7c6c629f (the previous main tip)
to df983ea1 (the new merge-ours commit). The vendored tree content is
byte-identical to 7c6c629f; only the commit graph is different.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant