fix(observability): avoid leaking Sentry setup secrets - #5489
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 23:23:04 UTC
⏸️ Suggested Action - Manual Review Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agent
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5489 +/- ##
=======================================
Coverage 94.72% 94.72%
=======================================
Files 559 559
Lines 44664 44664
Branches 14664 14664
=======================================
Hits 42308 42308
Misses 1621 1621
Partials 735 735
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…atasource.sh The PR only hardened setup-sentry-datasource.sh, but setup-github-datasource.sh has the byte-identical vulnerable pattern -- set -a-exported GITHUB_TOKEN/ GRAFANA_ADMIN_PASSWORD, an AUTH="admin:$PASSWORD" curl -u argv, and -d "$(payload)" embedding the token in argv too -- and both scripts' own header comments document them as siblings/templates for each other. Applies the identical hardening: a temp 0600 netrc file, a grafana_curl() wrapper that also strips both secrets from curl's child env via env -u, and piping the payload over stdin instead of argv. Verified end-to-end against a real local HTTP server (create + update + health-check all authenticate correctly via --netrc-file alone). Also adds this script's first-ever dedicated test file -- it previously had none.
Motivation
Description
.envby removingset -awhen sourcing.envso secrets are not put into child process environments.netrcfile and use--netrc-fileso Grafana admin credentials are not visible in process argv.grafana_curl()wrapper that callsenv -u GRAFANA_ADMIN_PASSWORD -u SENTRY_API_TOKEN curl ...to ensure those env vars are removed from curl's child environment.--data-binary @-(stdin) instead of-d "$(payload)"to keep the Sentry API token out of curl process arguments.test/unit/selfhost-grafana-sentry-datasource.test.tsthat verifies the script no longer containsset -a,-u "$AUTH", or-d "$(payload)"and that the new secure patterns (--netrc-file,--data-binary @-, andenv -u ... curl) are present.Testing
bash -n scripts/setup-sentry-datasource.shand it returned clean syntax (no errors).npx vitest run test/unit/selfhost-grafana-sentry-datasource.test.tsand the test file passed (10 tests total).GET/PUT/POSTand health-check continue to run while secrets are no longer exposed in argv or inherited child environments.Codex Task