diff --git a/.github/workflows/auto-release.yaml b/.github/workflows/auto-release.yaml index 1576ffdca..ddcca3fe3 100644 --- a/.github/workflows/auto-release.yaml +++ b/.github/workflows/auto-release.yaml @@ -50,11 +50,10 @@ jobs: # This run may have waited behind an earlier release. Take main and # the tags as they are now, so the version is computed after any # metadata pull request that landed while this run was queued. - # The checkout keeps no credentials; send the read token as a request - # header for this one fetch. - HEADER="$(printf 'x-access-token:%s' "${READ_TOKEN}" | base64 | tr -d '\n')" - echo "::add-mask::${HEADER}" - git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${HEADER}" fetch --force --tags origin main + # The checkout keeps no credentials; a credential helper hands git the + # read token from the environment for this one fetch. + # shellcheck disable=SC2016 # git expands GIT_AUTH_TOKEN when it runs the helper + GIT_AUTH_TOKEN="${READ_TOKEN}" git -c credential.helper= -c 'credential.helper=!f() { test "$1" = get && printf "username=x-access-token\npassword=%s\n" "${GIT_AUTH_TOKEN}"; }; f' fetch --force --tags origin main git checkout --detach origin/main echo "releasing from $(git rev-parse --short HEAD)" # Validation, tagging and publishing all use this commit, so the @@ -332,7 +331,7 @@ jobs: fetch-depth: 0 ref: ${{ needs.prepare-release.outputs.release_sha }} # The checkout only reads. The bot token is handed to the one step that - # pushes, as a request header, and never lands in the git config. + # pushes, through a credential helper, and never lands in the git config. persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 @@ -400,14 +399,14 @@ jobs: echo "::error::refusing to tag '${TAG}': not vMAJOR.MINOR.PATCH" exit 1 fi - # Run git against github.com with a token sent as a request header for - # this one command. The token is never put in a URL or in the git config. + # Run git against github.com with a token that a credential helper reads + # from the environment for this one command. The token is never put in + # a URL, in the git config or in a derived value that needs masking. git_as() { - local token="$1" header + local token="$1" shift - header="$(printf 'x-access-token:%s' "${token}" | base64 | tr -d '\n')" - echo "::add-mask::${header}" - git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${header}" "$@" + # shellcheck disable=SC2016 # git expands GIT_AUTH_TOKEN when it runs the helper + GIT_AUTH_TOKEN="${token}" git -c credential.helper= -c 'credential.helper=!f() { test "$1" = get && printf "username=x-access-token\npassword=%s\n" "${GIT_AUTH_TOKEN}"; }; f' "$@" } PRIMARY_TOKEN="${RELEASE_PR_TOKEN:-$WORKFLOW_TOKEN}" PRIMARY_ACTOR="RELEASE_PR_TOKEN" diff --git a/.github/workflows/perf-daily.yaml b/.github/workflows/perf-daily.yaml index 1c9456ae0..0aaf7903b 100644 --- a/.github/workflows/perf-daily.yaml +++ b/.github/workflows/perf-daily.yaml @@ -121,8 +121,8 @@ jobs: ref: main # The baseline is the newest release tag; it builds from a worktree. fetch-depth: 0 - # The checkout only reads. The step that pushes gets the bot token as - # a request header and nothing keeps a token in the git config. + # The checkout only reads. The step that pushes gets the bot token through + # a credential helper and nothing keeps a token in the git config. persist-credentials: false - name: Carry history an unmerged daily PR already holds @@ -133,9 +133,9 @@ jobs: # branch's copy is main's plus the days not merged yet. Only the # lines: ratchet.json comes from main (a row removed there by hand # must not come back) plus what this run adds. - HEADER="$(printf 'x-access-token:%s' "${READ_TOKEN}" | base64 | tr -d '\n')" - echo "::add-mask::${HEADER}" - if git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${HEADER}" fetch --no-tags origin "$BRANCH" 2>/dev/null; then + # A credential helper hands git the read token from the environment. + # shellcheck disable=SC2016 # git expands GIT_AUTH_TOKEN when it runs the helper + if GIT_AUTH_TOKEN="${READ_TOKEN}" git -c credential.helper= -c 'credential.helper=!f() { test "$1" = get && printf "username=x-access-token\npassword=%s\n" "${GIT_AUTH_TOKEN}"; }; f' fetch --no-tags origin "$BRANCH" 2>/dev/null; then git checkout FETCH_HEAD -- ':(glob)bench/ab/history/*.jsonl' || true fi @@ -203,11 +203,10 @@ jobs: PUSH_TOKEN: ${{ secrets.RELEASE_PR_TOKEN != '' && secrets.RELEASE_PR_TOKEN || github.token }} run: | set -euo pipefail - # Run git against github.com with the token sent as a request header - # for this one command. The token is never put in a URL or in the git - # config, and the checkout above did not keep one. - HEADER="$(printf 'x-access-token:%s' "${PUSH_TOKEN}" | base64 | tr -d '\n')" - echo "::add-mask::${HEADER}" + # The push runs git against github.com with the token that a credential + # helper reads from the environment for that one command. The token is + # never put in a URL or in the git config, and the checkout above did + # not keep one. # bench/ab/history includes trend.md and ratchet.json. FILES=(bench/ab/results bench/ab/history conformance/registry/generated/perf-evidence.json conformance/reports/performance.md conformance/reports/gaps.md) @@ -228,7 +227,8 @@ jobs: git checkout -B "$BRANCH" git add -- "${FILES[@]}" git commit -m "bench: daily performance report ${DATE}" - git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${HEADER}" push origin "$BRANCH" --force + # shellcheck disable=SC2016 # git expands GIT_AUTH_TOKEN when it runs the helper + GIT_AUTH_TOKEN="${PUSH_TOKEN}" git -c credential.helper= -c 'credential.helper=!f() { test "$1" = get && printf "username=x-access-token\npassword=%s\n" "${GIT_AUTH_TOKEN}"; }; f' push origin "$BRANCH" --force TITLE="bench: daily performance report ${DATE}" BODY="$(printf '%s\n\n%s\n\n%s\n\n%s\n' \ "$ALERTS" \ diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index d0b213190..605126b20 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -47,7 +47,7 @@ jobs: # Only an existing tag is released: the ref is spelled as a tag so a branch # of the same name can never be picked up. The checkout does not keep the - # token; the one step that pushes gets the bot token as a request header. + # token; the one step that pushes gets the bot token through a credential helper. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -488,14 +488,13 @@ jobs: PUSH_TOKEN: ${{ secrets.RELEASE_PR_TOKEN != '' && secrets.RELEASE_PR_TOKEN || github.token }} run: | set -euo pipefail - # Run git against github.com with the token sent as a request header - # for this one command. The token is never put in a URL or in the git - # config, and the checkout above did not keep one. + # Run git against github.com with the token that a credential helper + # reads from the environment for this one command. The token is never + # put in a URL or in the git config, and the checkout above did not + # keep one. git_as() { - local header - header="$(printf 'x-access-token:%s' "${PUSH_TOKEN}" | base64 | tr -d '\n')" - echo "::add-mask::${header}" - git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${header}" "$@" + # shellcheck disable=SC2016 # git expands GIT_AUTH_TOKEN when it runs the helper + GIT_AUTH_TOKEN="${PUSH_TOKEN}" git -c credential.helper= -c 'credential.helper=!f() { test "$1" = get && printf "username=x-access-token\npassword=%s\n" "${GIT_AUTH_TOKEN}"; }; f' "$@" } if [ "${HAS_RELEASE_PR_TOKEN}" != "true" ]; then echo "::warning::RELEASE_PR_TOKEN is not configured. The metadata branch is pushed with the job token and no pull request is opened for it." diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f2d1e518..0fc53ddc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- **The release and daily performance workflows no longer derive a token value + that has to be masked.** The steps that fetch or push with a token built a + base64 `Authorization` header from it and hid that header with + `::add-mask::`, which a step that stops workflow command processing can + defeat (Semgrep `unsafe-add-mask-workflow-command`, code-scanning alerts + 626–630). Git now gets the token from a credential helper that reads it from + the environment for that one command. The token is a repository secret or the + job token, which GitHub masks itself, and it is still never put in a URL or in + the git config. +- **The Semgrep `iterate-over-empty-map` finding in the unwrap conversion check + is fixed in the code instead of suppressed.** GitHub code scanning reopened it + (alert 631) despite the in-source `nosemgrep` marker added for alert 607, so + the loop that copies the names `| json` extracted is now `maps.Copy`, which + behaves the same because the map only holds `true` values. + ## [2.24.3] - 2026-10-21 ### Breaking Changes diff --git a/internal/proxy/unwrap_conversion_error.go b/internal/proxy/unwrap_conversion_error.go index bceed9a23..d8eb5a65c 100644 --- a/internal/proxy/unwrap_conversion_error.go +++ b/internal/proxy/unwrap_conversion_error.go @@ -967,14 +967,13 @@ func unwrapParseLine(stage *logqlpkg.ParserStage, line string, labels map[string return false } plan := &orderedJSONMetricPlan{preserveError: preserveError} - extracted := map[string]bool{} // nosemgrep: trailofbits.go.iterate-over-empty-map.iterate-over-empty-map -- parseJSON fills it before the loop below + extracted := map[string]bool{} before := cloneStringMap(labels) if err := plan.parseJSON(line, before, labels, extracted); err != nil { return false } - for name := range extracted { - parsed[name] = true - } + // parseJSON stores only true values. + maps.Copy(parsed, extracted) case logqlpkg.ParserLogfmt: if stage.Param != "" || len(stage.Fields) != 0 { return false diff --git a/internal/proxy/unwrap_conversion_error_test.go b/internal/proxy/unwrap_conversion_error_test.go index 9864c8e50..d1e2221ac 100644 --- a/internal/proxy/unwrap_conversion_error_test.go +++ b/internal/proxy/unwrap_conversion_error_test.go @@ -2,6 +2,7 @@ package proxy import ( "encoding/json" + "maps" "net/http" "net/http/httptest" "net/url" @@ -526,3 +527,25 @@ func TestUnwrapAcceptablePatternCoversConversions(t *testing.T) { } } } + +// A name `| json` extracted counts as parsed for the stages after it, so a +// later parser neither replaces it nor adds it again with the _extracted +// suffix, which is kept for stream labels. +func TestUnwrapParseLineJSONNamesStayParsedForLaterStages(t *testing.T) { + line := `{"level":"info"}` + labels := map[string]string{"app": "x"} + parsed := map[string]bool{} + stages := []*logqlpkg.ParserStage{ + {Type: logqlpkg.ParserJSON}, + {Type: logqlpkg.ParserRegexp, Param: `"(?Pl[a-z]+)"`}, + } + for _, stage := range stages { + if !unwrapParseLine(stage, line, labels, parsed, false) { + t.Fatalf("stage %v was not reproduced", stage.Type) + } + } + want := map[string]string{"app": "x", "level": "info"} + if !maps.Equal(labels, want) { + t.Fatalf("labels = %v, want %v", labels, want) + } +}