fix: disable telemetry when send-statistics is false - #135
Conversation
📝 WalkthroughWalkthroughThe action now assigns ChangesTelemetry opt-out
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Line 82: Update checkFlake() so process.env is spread into executionEnv before
the action-specific assignments, ensuring NIX_FLAKE_CHECKER_NO_TELEMETRY remains
"true" even when inherited environment variables define a conflicting value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cd15deb0-a23e-4446-9f6e-8338c69fa968
⛔ Files ignored due to path filters (2)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (1)
src/index.ts
|
|
||
| if (!this.sendStatistics) { | ||
| executionEnv.NIX_FLAKE_CHECKER_NO_TELEMETRY = "false"; | ||
| executionEnv.NIX_FLAKE_CHECKER_NO_TELEMETRY = "true"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Ensure the action value cannot be overridden by process.env.
Line 82 sets the correct value. However, checkFlake() spreads process.env after executionEnv on Lines 55-56. An inherited NIX_FLAKE_CHECKER_NO_TELEMETRY="false" therefore overrides this assignment and keeps telemetry enabled.
Reverse the spread order so the action configuration has precedence:
Proposed fix
env: {
- ...executionEnv,
...process.env,
+ ...executionEnv,
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.ts` at line 82, Update checkFlake() so process.env is spread into
executionEnv before the action-specific assignments, ensuring
NIX_FLAKE_CHECKER_NO_TELEMETRY remains "true" even when inherited environment
variables define a conflicting value.
Fixes #134
Summary by CodeRabbit