fix: use COPILOT_DUMMY_BYOK indirection to suppress secret-scanner false positives on lock files#33116
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…YOK indirection Secret scanners flag generated lock files when COPILOT_API_KEY contains a token-shaped literal value. Instead, introduce a new COPILOT_DUMMY_BYOK env var that holds the sentinel, and set COPILOT_API_KEY to reference it via $COPILOT_DUMMY_BYOK. COPILOT_DUMMY_BYOK has no *_API_KEY suffix so scanners won't flag it; the COPILOT_API_KEY line holds a variable reference rather than a literal, which also avoids triggering pattern-based scanners. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adjusts Copilot lock-file emission to avoid secret-scanner false positives caused by writing a token-shaped literal next to COPILOT_API_KEY in generated .lock.yml workflows.
Changes:
- Adds
CopilotBYOKDummyAPIKeyEnvVarconstant (COPILOT_DUMMY_BYOK) to carry the dummy sentinel under a non-*_API_KEYname. - Updates Copilot engine step env generation to emit
COPILOT_API_KEY: $COPILOT_DUMMY_BYOKandCOPILOT_DUMMY_BYOK: dummy-byok-key-for-offline-mode. - Updates unit tests, golden fixtures, and committed workflow lock files to match the new pattern.
Show a summary per file
| File | Description |
|---|---|
| pkg/constants/engine_constants.go | Adds a new constant for the dummy BYOK env var indirection. |
| pkg/workflow/copilot_engine_execution.go | Changes Copilot step env emission to use an indirection variable. |
| pkg/workflow/copilot_engine_test.go | Updates tests to assert the new two-variable pattern. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/basic-copilot.golden | Updates golden output to include COPILOT_DUMMY_BYOK and reference it from COPILOT_API_KEY. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/playwright-cli-mode.golden | Same golden fixture update for Playwright CLI mode. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/smoke-copilot.golden | Same golden fixture update for smoke Copilot compile fixture. |
| pkg/workflow/testdata/TestWasmGolden_CompileFixtures/with-imports.golden | Same golden fixture update for import-based fixture. |
| .github/workflows/ab-testing-advisor.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/ace-editor.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/agent-performance-analyzer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/agent-persona-explorer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/archie.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/architecture-guardian.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/artifacts-summary.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/auto-triage-issues.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/aw-portfolio-yield.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection (and includes additional unrelated lock-output changes). |
| .github/workflows/bot-detection.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/brave.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/breaking-change-checker.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/chaos-pr-bundle-fuzzer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/ci-coach.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/cli-consistency-checker.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/code-scanning-fixer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/code-simplifier.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/constraint-solving-potd.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/contribution-check.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-opt.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-pr-merged-report.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-token-audit.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/copilot-token-optimizer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/craft.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-agent-of-the-day-blog-writer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-architecture-diagram.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-assign-issue-to-user.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-cli-performance.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-cli-tools-tester.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-community-attribution.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-compiler-quality.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-compiler-threat-spec-optimizer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-experiment-report.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-file-diet.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-firewall-report.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-geo-optimizer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-hippo-learn.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-issues-report.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-malicious-code-scan.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-mcp-concurrency-analysis.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-model-inventory.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-news.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-performance-summary.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-regulatory.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-repo-chronicle.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-safe-output-integrator.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-secrets-analysis.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-security-observability.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-semgrep-scan.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-sentrux-report.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-skill-optimizer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-spdd-spec-planner.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-syntax-error-quality.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-team-status.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/daily-workflow-updater.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dead-code-remover.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/delight.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dependabot-burner.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dependabot-campaign.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dependabot-go-checker.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dependabot-repair.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dependabot-worker.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/deployment-incident-monitor.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dev-hawk.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/dictation-prompt.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/discussion-task-miner.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/docs-noob-tester.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/draft-pr-cleanup.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/example-permissions-warning.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/firewall-escape.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/firewall.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/functional-pragmatist.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/github-remote-mcp-auth-test.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/glossary-maintainer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/gpclean.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/hippo-embed.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/issue-monster.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/issue-triage-agent.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/jsweep.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/layout-spec-maintainer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/lint-monster.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/linter-miner.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/mattpocock-skills-reviewer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/mcp-inspector.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/mergefest.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/metrics-collector.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/notion-issue-summary.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/org-health-report.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/outcome-collector.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/pdf-summary.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/plan.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/poem-bot.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/pr-code-quality-reviewer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/pr-description-caveman.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/pr-nitpick-reviewer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/pr-sous-chef.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/pr-triage-agent.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/python-data-charts.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/q.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/refactoring-cadence.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/refiner.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/release.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/repo-audit-analyzer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/repo-tree-map.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/repository-quality-improver.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/research.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/security-compliance.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/security-review.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/slide-deck-maintainer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-ci.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-copilot-arm.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-copilot.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-create-cross-repo-pr.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-multi-pr.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-otel-backends.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-project.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-service-ports.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-temporary-id.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-test-tools.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-update-cross-repo-pr.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-workflow-call-with-inputs.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/smoke-workflow-call.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/spec-extractor.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/spec-librarian.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/stale-pr-cleanup.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/stale-repo-identifier.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/sub-issue-closer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/super-linter.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/technical-doc-writer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/terminal-stylist.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/test-dispatcher.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/test-project-url-default.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/test-quality-sentinel.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/test-workflow.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/tidy.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/ubuntu-image-analyzer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/uk-ai-operational-resilience.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/update-astro.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/video-analyzer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/visual-regression-checker.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/weekly-blog-post-writer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/weekly-editors-health-check.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/weekly-issue-summary.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/weekly-safe-outputs-spec-review.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/workflow-generator.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/workflow-health-manager.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/workflow-normalizer.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
| .github/workflows/workflow-skill-extractor.lock.yml | Updates generated env to use COPILOT_DUMMY_BYOK indirection. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 159/159 changed files
- Comments generated: 4
| if sandboxEnabled { | ||
| env["COPILOT_API_KEY"] = constants.CopilotBYOKDummyAPIKey | ||
| env[constants.CopilotBYOKDummyAPIKeyEnvVar] = constants.CopilotBYOKDummyAPIKey | ||
| env["COPILOT_API_KEY"] = "$" + constants.CopilotBYOKDummyAPIKeyEnvVar | ||
| env["AWF_REFLECT_ENABLED"] = "1" | ||
| } |
| // COPILOT_API_KEY must reference the variable, not the literal value. | ||
| expectedAPIKey := "COPILOT_API_KEY: $" + constants.CopilotBYOKDummyAPIKeyEnvVar | ||
| if !strings.Contains(stepContent, expectedAPIKey) { | ||
| t.Errorf("Expected COPILOT_API_KEY to reference $%s (not the literal), got:\n%s", constants.CopilotBYOKDummyAPIKeyEnvVar, stepContent) | ||
| } | ||
| // Sanity-check: the literal dummy key must NOT appear next to COPILOT_API_KEY. | ||
| if strings.Contains(stepContent, "COPILOT_API_KEY: "+constants.CopilotBYOKDummyAPIKey) { | ||
| t.Errorf("COPILOT_API_KEY must not contain the literal dummy key value; got:\n%s", stepContent) |
| // CopilotBYOKDummyAPIKeyEnvVar is the environment variable that holds the | ||
| // CopilotBYOKDummyAPIKey sentinel value in generated lock files. Using a | ||
| // non-_API_KEY-shaped name for the literal value prevents secret scanners from | ||
| // flagging the generated artifact. COPILOT_API_KEY is then set to reference | ||
| // this variable (e.g. $COPILOT_DUMMY_BYOK) so the value is never written |
| env: | ||
| OTEL_EXPORTER_OTLP_ENDPOINT: ${{ secrets.GH_AW_OTEL_SENTRY_ENDPOINT }} | ||
| OTEL_SERVICE_NAME: gh-aw | ||
| OTEL_EXPORTER_OTLP_HEADERS: Authorization=${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }} | ||
| GH_AW_OTLP_ALL_HEADERS: Authorization=${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }},Authorization=${{ secrets.GH_AW_OTEL_GRAFANA_AUTHORIZATION }} | ||
| GH_AW_OTLP_ENDPOINTS: '[{"url":"${{ secrets.GH_AW_OTEL_SENTRY_ENDPOINT }}","headers":"Authorization=${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }}"},{"url":"${{ secrets.GH_AW_OTEL_GRAFANA_ENDPOINT }}","headers":"Authorization=${{ secrets.GH_AW_OTEL_GRAFANA_AUTHORIZATION }}"}]' | ||
| OTEL_SERVICE_NAME: gh-aw.aw-portfolio-yield | ||
| OTEL_EXPORTER_OTLP_HEADERS: x-sentry-auth=${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }} | ||
| GH_AW_OTLP_ALL_HEADERS: x-sentry-auth=${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }},Authorization=${{ secrets.GH_AW_OTEL_GRAFANA_AUTHORIZATION }} | ||
| GH_AW_OTLP_ENDPOINTS: '[{"url":"${{ secrets.GH_AW_OTEL_SENTRY_ENDPOINT }}","headers":"x-sentry-auth=${{ secrets.GH_AW_OTEL_SENTRY_AUTHORIZATION }}"},{"url":"${{ secrets.GH_AW_OTEL_GRAFANA_ENDPOINT }}","headers":"Authorization=${{ secrets.GH_AW_OTEL_GRAFANA_AUTHORIZATION }}"}]' |
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification DetailsView Test Classification Table
AnalysisThe PR modifies
All assertions use Minor note: The assertions use Test inflation: +17 test lines vs +16 production lines — ratio ≈ 1:1. No inflation. Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose — this is a targeted bug fix for secret-scanner false positives on generated lock files.
Key Themes
- Root cause addressed: The fix correctly identifies that scanners flag literal token-shaped values adjacent to
*_API_KEYkeys, and resolves it by introducing an indirection variable with a non-sensitive-looking name. - Runtime semantics gap: GitHub Actions
env:blocks do not perform shell variable expansion, soCOPILOT_API_KEYreceives the literal string$COPILOT_DUMMY_BYOKat runtime — not the sentinel value. This is functionally correct (the AWF proxy intercepts before key validation), but it's subtle and undocumented in the code itself. - Heuristic brittleness: The fix is correct today but future scanner updates could catch
COPILOT_DUMMY_BYOK: dummy-byok-key-for-offline-modetoo — worth a comment on the constant naming constraint.
Positive Highlights
- ✅ Clear, well-documented PR description explaining the before/after
- ✅ Tests updated with three distinct assertions: literal in indirection var, reference in API_KEY, and negative check that literal is never adjacent to API_KEY
- ✅ Constant is properly named and placed in
engine_constants.go - ✅ All 159 generated lock files regenerated mechanically — no manual edits
- ✅ Golden fixtures updated alongside tests
Verdict
No blocking issues — both inline comments are observations/suggestions, not correctness problems. The fix does what it says and the test coverage is solid.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 6.5M
| if sandboxEnabled { | ||
| env["COPILOT_API_KEY"] = constants.CopilotBYOKDummyAPIKey | ||
| env[constants.CopilotBYOKDummyAPIKeyEnvVar] = constants.CopilotBYOKDummyAPIKey | ||
| env["COPILOT_API_KEY"] = "$" + constants.CopilotBYOKDummyAPIKeyEnvVar |
There was a problem hiding this comment.
[/diagnose] The comment explains the intent but not the actual runtime semantics: because GitHub Actions does not perform shell variable expansion inside env: blocks, COPILOT_API_KEY will receive the literal string $COPILOT_DUMMY_BYOK in the process environment — not the value dummy-byok-key-for-offline-mode.\n\nThis still works because the AWF API proxy intercepts all Copilot CLI requests before any real key validation, so any non-empty placeholder is fine. But a future reader might be surprised. A one-line comment would prevent well-meaning fixes:\n\ngo\n// Note: GitHub Actions env: blocks do not expand $VAR references, so the process\n// receives the literal string "$COPILOT_DUMMY_BYOK". Any non-empty placeholder\n// works here because the AWF proxy intercepts before any real key check.\n
| // this variable (e.g. $COPILOT_DUMMY_BYOK) so the value is never written | ||
| // inline next to a *_API_KEY key. | ||
| CopilotBYOKDummyAPIKeyEnvVar = "COPILOT_DUMMY_BYOK" | ||
|
|
There was a problem hiding this comment.
[/diagnose] Minor resilience note: the fix works against today's scanner heuristics (literal token-shaped value adjacent to a *_API_KEY key), but it's a cat-and-mouse approach. If a scanner is later updated to also flag COPILOT_DUMMY_BYOK: dummy-byok-key-for-offline-mode as suspicious, the false positive returns. This is worth noting in a comment alongside the constant so future maintainers know the constraint driving the naming choice — specifically that the variable name must not end in _API_KEY, _SECRET, or similar patterns.
There was a problem hiding this comment.
✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new/modified tests are implementation tests (threshold: 30%). The updated assertions enforce the behavioral contract that COPILOT_API_KEY must reference the indirection variable rather than the literal dummy key value.
|
@copilot address the review feedback |
…ntime expansion GitHub Actions env: blocks are NOT shell-expanded, so setting COPILOT_API_KEY: $COPILOT_DUMMY_BYOK would pass the literal string "$COPILOT_DUMMY_BYOK" to the process instead of the sentinel value. Instead, add 'export COPILOT_API_KEY="$COPILOT_DUMMY_BYOK"' to pathSetup (the host-side shell script that runs before sudo -E awf). Shell variable expansion runs correctly there, and sudo -E preserves COPILOT_API_KEY for the AWF container. COPILOT_DUMMY_BYOK remains in the env: block (no *_API_KEY suffix), and COPILOT_API_KEY never appears as a YAML env: key. Also fix CopilotBYOKDummyAPIKeyEnvVar comment wording: "non-_API_KEY-shaped" -> "non-*_API_KEY-shaped" Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed in the latest commit. The approach is now:
Also fixed the |
|
@pelikhan make sure this looks ok to you |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✅ Smoke OTEL completed successfully! |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨ Caution agentic threat detected Smoke test completed with some failures. |
|
🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧 |
Agent Container Tool Check
Result: 11/12 tools available
|
|
Caution agentic threat detected Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
|
Commit pushed:
|
|
Smoke Test: Codex 26056780277 — FAIL Warning Firewall blocked 4 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "clients2.google.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
💥 Smoke Test: Claude — Run 26056780378Core #1-12: ✅ all pass · PR review #13-15,17,18: ✅ · #16,19: Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
💥 [THE END] — Illustrated by Smoke Claude · ● 6M
|
🦴 UGH! Caveman smoke test agent was HERE! Me run tests. Tests PASS. Tools WORK. Very good! 🔥 Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
|
Smoke Test 26056780184 — Results for
Overall: Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Two inline comments left on pkg/constants/engine_constants.go. The approach of using COPILOT_DUMMY_BYOK indirection is sound and cleanly avoids secret-scanner false positives. The test and fixture updates look complete.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · ● 20.2M
| // flagging the generated artifact. COPILOT_API_KEY is then exported in the run: | ||
| // shell script via shell variable expansion so the value is never written | ||
| // inline next to a *_API_KEY key in the YAML env: block. | ||
| CopilotBYOKDummyAPIKeyEnvVar = "COPILOT_DUMMY_BYOK" |
There was a problem hiding this comment.
Good use of indirection! COPILOT_DUMMY_BYOK avoids the *_API_KEY suffix that triggers secret scanners — clean fix.
| // isolated in the AWF API proxy sidecar. | ||
| CopilotBYOKDummyAPIKey = "dummy-byok-key-for-offline-mode" | ||
|
|
||
| // CopilotBYOKDummyAPIKeyEnvVar is the environment variable that holds the |
There was a problem hiding this comment.
The comment block clearly explains the two-variable pattern. Consider also noting that this is a build-time constant so the env var name is stable.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
|
Generated
.lock.ymlfiles emittedCOPILOT_API_KEY: dummy-byok-key-for-offline-mode— a literal token-shaped value next to an*_API_KEYkey — causing secret scanners to flag committed lock files as containing credentials.Changes
CopilotBYOKDummyAPIKeyEnvVar = "COPILOT_DUMMY_BYOK"inpkg/constants/engine_constants.go— a non-*_API_KEY-shaped name to hold the sentinelcopilot_engine_execution.go): instead ofCOPILOT_API_KEY: dummy-byok-key-for-offline-mode, the compiler now emits two variables:COPILOT_DUMMY_BYOKcarries the literal but has no*_API_KEYsuffix;COPILOT_API_KEYholds a variable reference rather than a token-shaped literal — neither form triggers scanner rules.COPILOT_API_KEY.lock.ymlworkflow filesChangeset\n\n- Type: patch\n- Description: Fix generated lock files to avoid secret-scanner false positives by routing the dummy Copilot API key through indirection instead of emitting the literal token-shaped value inline.
✨ PR Review Safe Output Test - Run 26056780378
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.comSee Network Configuration for more information.