Skip to content

Commit 7c6c629

Browse files
msukkariclaude
andauthored
chore: resolve open CodeQL security alerts (#13)
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>
1 parent 1061b7b commit 7c6c629

6 files changed

Lines changed: 35 additions & 8 deletions

File tree

.github/workflows/buf-breaking-check.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ on:
44
- opened
55
paths:
66
- '*.proto'
7+
permissions:
8+
contents: read
79
jobs:
810
validate-protos:
911
runs-on: ubuntu-latest

.github/workflows/ci.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ on:
55
pull_request:
66
workflow_dispatch:
77
name: CI
8+
permissions:
9+
contents: read
810
jobs:
911
test:
1012
runs-on: ubuntu-latest

.github/workflows/semgrep.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
name: Semgrep - SAST Scan
22

33
on:
4-
pull_request_target:
4+
pull_request:
55
types: [ closed, edited, opened, synchronize, ready_for_review ]
66

7+
permissions:
8+
contents: read
9+
710
jobs:
811
semgrep:
912
permissions:
@@ -16,9 +19,6 @@ jobs:
1619

1720
steps:
1821
- uses: actions/checkout@v4
19-
with:
20-
ref: ${{ github.event.pull_request.head.ref }}
21-
repository: ${{ github.event.pull_request.head.repo.full_name }}
2222

2323
- name: Checkout semgrep-rules repo
2424
uses: actions/checkout@v4

api.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -664,8 +664,8 @@ func (r *Repository) UnmarshalJSON(data []byte) error {
664664
}
665665

666666
if v, ok := repo.RawConfig["tenantID"]; ok {
667-
id, _ := strconv.ParseInt(v, 10, 64)
668-
r.TenantID = int(id)
667+
id, _ := strconv.Atoi(v)
668+
r.TenantID = id
669669
}
670670

671671
// Sourcegraph indexserver doesn't set repo.Rank, so we set it here. Setting it

cmd/zoekt-sourcegraph-indexserver/sg.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ func (sf sourcegraphFake) id(name string) uint32 {
603603
// allow overriding the ID.
604604
idPath := filepath.Join(sf.RootDir, filepath.FromSlash(name), "SG_ID")
605605
if b, _ := os.ReadFile(idPath); len(b) > 0 {
606-
id, err := strconv.Atoi(strings.TrimSpace(string(b)))
606+
id, err := strconv.ParseUint(strings.TrimSpace(string(b)), 10, 32)
607607
if err == nil {
608608
return uint32(id)
609609
}

gitindex/clone.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"bytes"
1919
"fmt"
2020
"log"
21+
"net/url"
2122
"os"
2223
"os/exec"
2324
"path/filepath"
@@ -26,6 +27,28 @@ import (
2627
"github.com/go-git/go-git/v5/config"
2728
)
2829

30+
// redactURLCredentials removes any basic-auth userinfo from a URL-shaped
31+
// string so that credentials are never written to logs. Non-URL arguments
32+
// are returned unchanged.
33+
func redactURLCredentials(s string) string {
34+
u, err := url.Parse(s)
35+
if err != nil || u.Scheme == "" || u.User == nil {
36+
return s
37+
}
38+
u.User = url.User("redacted")
39+
return u.String()
40+
}
41+
42+
// redactURLCredentialsInArgs returns a copy of args with any URL-valued
43+
// arguments having their userinfo redacted.
44+
func redactURLCredentialsInArgs(args []string) []string {
45+
out := make([]string, len(args))
46+
for i, a := range args {
47+
out[i] = redactURLCredentials(a)
48+
}
49+
return out
50+
}
51+
2952
// CloneRepo clones one repository, adding the given config
3053
// settings. It returns the bare repo directory. The `name` argument
3154
// determines where the repo is stored relative to `destDir`. Returns
@@ -57,7 +80,7 @@ func CloneRepo(destDir, name, cloneURL string, settings map[string]string) (stri
5780

5881
// Prevent prompting
5982
cmd.Stdin = &bytes.Buffer{}
60-
log.Println("running:", cmd.Args)
83+
log.Println("running:", redactURLCredentialsInArgs(cmd.Args))
6184
if err := cmd.Run(); err != nil {
6285
return "", err
6386
}

0 commit comments

Comments
 (0)