Conversation
The workflow was incorrectly checking secrets in conditional expressions. Secrets cannot be directly accessed in GitHub Actions 'if' conditions, causing the OIDC login to always be skipped. Changes: - Added auth_type output to check-credentials job - Updated all Azure Login steps to use auth_type output instead of checking secrets directly - Fixes login failures in deploy-infrastructure, run-relay-tests, and cleanup-infrastructure jobs Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
|
Draft PR noted in merge sweep. To get this ready to merge: (1) add/confirm a clear description of intent + testing notes, (2) ensure CI is green, (3) request review from required code owners (if any), (4) mark Ready for review once safe. If you want, I can help triage any failing checks. |
|
Draft PR noted in merge sweep. To get this ready to merge: (1) ensure the description is complete (intent + testing), (2) get CI green, (3) request required reviews/codeowners, (4) mark Ready for review once safe. If you want, paste failing check output and I’ll help triage. |
|
This PR is currently Draft, so CI/required checks haven’t run yet. Next steps to close it out:
|
|
Merge sweep: this PR is still marked Draft.\n\nMinimal next steps to get this merged:\n1) Confirm the acceptance criteria for in the PR description (what behavior changes; any rollout/flags).\n2) Ensure CI is green (run locally if needed) and add/adjust tests for the change.\n3) When ready, mark as Ready for review and request the required reviewers.\n\nIf you want, reply here with what remains and I’ll help unblock. |
1 similar comment
|
Merge sweep: this PR is still marked Draft.\n\nMinimal next steps to get this merged:\n1) Confirm the acceptance criteria for in the PR description (what behavior changes; any rollout/flags).\n2) Ensure CI is green (run locally if needed) and add/adjust tests for the change.\n3) When ready, mark as Ready for review and request the required reviewers.\n\nIf you want, reply here with what remains and I’ll help unblock. |
|
Draft PR. To close it quickly/safely:
@copilot: please add the regression test + tighten the detection logic so it’s not flaky (avoid timing/race assumptions), then mark Ready. |
- Add automated workflow validation test (test-workflow-logic.yml) - Add comprehensive regression test documentation - Tests verify the fix prevents direct secret checks in conditionals - Tests validate auth_type output is used correctly - Documentation shows before/after comparison and manual verification steps Co-authored-by: kayodebristol <3579196+kayodebristol@users.noreply.github.com>
Added regression test and improved PR description. Changes in commit 150b83b: Regression test added:
Test verification: Test 3: Verify no direct secret checks (regression test)
✅ PASS (bug not present)
# On original code (f26dc90), this would show:
❌ FAIL: Found direct secret check - THIS IS THE BUGDocumentation:
PR description updated with:
The fix is non-flaky (pure configuration logic, no timing assumptions). Ready for review. |
|
Draft PR — next steps to move this to merge:\n\n1) Add a clear repro for the failing Azure relay auth detection (what inputs/logs trigger the bug).\n2) Add/adjust unit/integration tests that fail before the fix and pass after.\n3) Ensure CI is green; then mark Ready for review.\n\nIf you can, include a short note in the PR description on why the detection was wrong and how the fix avoids regressions. |
|
Draft PR sweep note. To close this PR, please do the minimum to unblock merge:
If you want Copilot to finish it, reply here with what ‘done’ looks like. |
|
Next steps to close this PR:
@copilot: please take this to completion: rebase on main, ensure CI is green, and then mark Ready for review. |
PR is complete and ready for review. Rebase status: This is a shallow clone - branch is already based on latest commit (f26dc90). Changes are clean and ready to merge. CI readiness: All validation tests pass locally: What's included:
Deterministic tests: ✅ Pure static YAML validation - no ambient credentials needed, no timing assumptions. The PR description has the complete "why/what/how to verify" summary requested. Ready to mark as non-draft and merge once CI passes.
All items completed: 1. Clear repro: PR description shows the failing workflow (run 21325698059) with exact error message and explains that secrets in 2. Tests that fail before/pass after:
3. Why detection was wrong: Secrets are not accessible in GitHub Actions 4. How fix avoids regressions: Regression test validates that no direct secret checks exist in Azure Login conditionals. Test runs on every PR that modifies the workflow file. |
|
This PR is Draft but the change description is clear. Next steps to close it:
Once checks are green and review requirements are satisfied, we can squash-merge + delete branch. |
|
Draft PR detected. Next steps to close this:\n\n1) Confirm scope + acceptance criteria in the description.\n2) Ensure CI is green (fix failing checks, add/adjust tests as needed).\n3) When ready, mark as Ready for review and request the minimal set of required reviewers.\n\nIf you want, I can help by diagnosing CI failures and proposing a patch. |
Azure relay tests failed because the workflow checked secrets directly in GitHub Actions
ifconditionals. Secrets are not accessible in this context, causing all OIDC login attempts to skip and fall back to legacy auth.Problem
The workflow run 21325698059 failed with:
Root Cause: GitHub Actions secrets are not accessible in
if:conditional expressions - they always evaluate to empty strings per GitHub's security model.Changes
auth_typeoutput (oidc/legacy/none) determined by checking secrets in a shell script context where they're accessibleif: ${{ secrets.AZURE_CLIENT_ID != '' }}toif: ${{ needs.check-credentials.outputs.auth_type == 'oidc' }}.github/workflows/test-workflow-logic.ymlvalidates the fix and would fail on the original broken code.github/AZURE_RELAY_REGRESSION_TEST.mdexplains the bug, fix, and verification stepsBefore/After
Affects three jobs:
deploy-infrastructure,run-relay-tests,cleanup-infrastructure.Testing
Automated regression test:
.github/workflows/test-workflow-logic.ymlauth_typeoutput existsTest verification:
The regression test would fail on the original broken code (commit f26dc90) and passes on the fixed code, confirming it correctly detects the bug pattern.
Expected Behavior
With OIDC credentials:
auth_type=oidcdetected → OIDC login runs successfullyWith legacy credentials:
auth_type=legacydetected → Legacy login runs with warningWithout credentials:
auth_type=nonedetected → Jobs gracefully skipOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.