fix(observability): keep GitHub datasource credentials out of curl argv and child env - #5503
fix(observability): keep GitHub datasource credentials out of curl argv and child env#5503JSONbored wants to merge 1 commit into
Conversation
…gv and child env setup-github-datasource.sh had the identical secret-leak pattern PR #5489 fixed in its sibling setup-sentry-datasource.sh: sourcing .env with `set -a` exported GITHUB_TOKEN/GRAFANA_ADMIN_PASSWORD into every child process's environment, `curl -u "$AUTH"` put GRAFANA_ADMIN_PASSWORD on the process argv (visible to any local user via ps), and `-d "$(payload)"` put the GitHub token in argv too. Apply the same fix: drop `set -a` on the .env source, authenticate via a temporary permissioned netrc file instead of `-u`, pipe the JSON payload to curl over stdin via `--data-binary @-` instead of `-d`, and wrap curl in a `grafana_curl()` helper that additionally unsets GRAFANA_ADMIN_PASSWORD and GITHUB_TOKEN from its own environment as defense in depth. Idempotent update-vs-create and the health check are unchanged. Adds test/unit/selfhost-grafana-github-datasource.test.ts (no prior dedicated test file existed for this script) with a regression test mirroring the one added for the Sentry script in #5489, plus mode and behavior-preservation checks.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Closing as redundant — this is already shipped. #5489 (merged) fixed the identical Good find either way — no action needed, already covered. |
Summary
scripts/setup-github-datasource.shhad the identical secret-leak pattern that PR fix(observability): avoid leaking Sentry setup secrets #5489 fixed in its siblingscripts/setup-sentry-datasource.sh: sourcing.envwithset -aexportedGITHUB_TOKEN/GRAFANA_ADMIN_PASSWORDinto every child process's environment,curl -u "$AUTH"putGRAFANA_ADMIN_PASSWORDon the process argv (visible to any local user viaps aux), and-d "$(payload)"put the GitHub token in argv too.set -aon the.envsource (plain. ./.env), authenticate via a temporary permissionednetrcfile with--netrc-fileinstead of-u "$AUTH", pipe the JSON payload to curl over stdin via--data-binary @-instead of-d "$(payload)", and wrap curl in agrafana_curl()helper that additionally doesenv -u GRAFANA_ADMIN_PASSWORD -u GITHUB_TOKENas defense in depth.setup-github-datasource.shbefore this PR (only two incidental cross-references from the Sentry script's own test file). Addedtest/unit/selfhost-grafana-github-datasource.test.tswith a regression test mirroring the one fix(observability): avoid leaking Sentry setup secrets #5489 added for the Sentry script, plus an executable-mode check and a behavior-preservation check (idempotent PUT/POST,secureJsonData/accessToken, health check all still present).Scope
scripts/,test/), narrow, single coherent changeValidation
bash -n scripts/setup-github-datasource.sh— clean syntaxnpx vitest run test/unit/selfhost-grafana-github-datasource.test.ts test/unit/selfhost-grafana-sentry-datasource.test.ts— 12/12 passingnpm run test:ci— full local gate green (794 test files / 15408 tests passed, 12 skipped)npm audit --audit-level=moderate— 0 vulnerabilitiesSafety
0o100, verified by the new test)scripts/andtest/are Codecov-exempt per this repo'scodecov.yml(src/**only) — no patch-coverage obligation, but ran the full gate anywayFollow-up to #5489 (same author/account, same bug class, same fix pattern) — no separate tracking issue exists for this narrow mirrored fix, matching #5489's own precedent (also unlinked).