feat(azdext): add security validation and ssrf guard#3
Closed
jongio wants to merge 10 commits intofeature/ext-pr3-6946from
Closed
feat(azdext): add security validation and ssrf guard#3jongio wants to merge 10 commits intofeature/ext-pr3-6946from
jongio wants to merge 10 commits intofeature/ext-pr3-6946from
Conversation
Implements Azure#6945 (P1-5/P1-6). Co-authored-by: Copilot <[email protected]>
Implements Azure#6946 (P2-1/P2-2). Co-authored-by: Copilot <[email protected]>
Implements Azure#6947 (P2-3/P2-4). Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Reuse shared SSRF host/CIDR and IP checks between guard and MCP policy to keep behavior aligned. Co-authored-by: Copilot <[email protected]>
- ssrf_guard: strengthen IP validation and blocklist enforcement - Propagate core fixes: mcp_security, pagination, resilient_http_client - Propagate helper fixes: config_helper, keyvault_resolver
…verflow - SSRFSafeRedirect: add DNS resolution for hostname redirects (fail-closed) to prevent bypass via attacker-controlled DNS pointing to private IPs - MCPSecurityPolicy.CheckURL: normalize IPv4-mapped IPv6 addresses before metadata host matching (blocks ::ffff:169.254.169.254 bypass) - SSRFGuard.Check: same IPv4-mapped normalization for standalone guard - retryAfterFromResponse: cap parsed value before multiplication to prevent integer overflow that could bypass maxRetryAfterDuration - keyvault_resolver: fix misleading error message about consecutive hyphens - Add regression tests for all hardened paths
This was referenced Mar 2, 2026
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Mar 5, 2026
- Prepend custom scope rules before defaults so overrides work (#1) - Redact URL query params in ScopeDetectorError to prevent leaking secrets (#2) - Add versioned User-Agent string, make configurable via ResilientClientOptions (#3) - Set done=true on Collect truncation to prevent surprise continuation (#4) - Add azdext SDK version constant (version.go) Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Mar 6, 2026
) * feat(azdext): add P1 core extension primitives Implements Azure#6944 core primitives for token provider, scope detection, resilient HTTP client, and pagination with tests. Co-authored-by: Copilot <[email protected]> * fix(azdext): harden P1 primitives after quality review Addresses MQ findings for Azure#6944: bounded response reads, nextLink SSRF protections, retry/body semantics, token-over-http guard, deterministic scope rules, and added regression tests. Co-authored-by: Copilot <[email protected]> * chore: fix preflight blockers for PR1 Apply required gofmt and cspell updates so mage preflight passes for draft PR Azure#6954. Co-authored-by: Copilot <[email protected]> * security: harden core primitives against hack scan findings - mcp_security: tighten input validation and error handling - pagination: add bounds checking on page parameters - resilient_http_client: strengthen TLS config and timeout enforcement - resilient_http_client_test: add security-path test coverage * fix: address profile review findings for stacked PR Co-authored-by: Copilot <[email protected]> * fix(azdext): satisfy lint and cspell checks Co-authored-by: Copilot <[email protected]> * fix(azdext): remediate hack findings Co-authored-by: Copilot <[email protected]> * fix: address copilot review feedback on PR 6954 - block hostname redirects that resolve to private/loopback IPs\n- return explicit nil-client error in stdHTTPDoer path\n- honor MaxRetries=0 as no retries; use negative as default sentinel\n- update TokenProvider usage snippet to current API\n\nCo-authored-by: Copilot <[email protected]> * fix: address follow-up Copilot feedback on PR 6954 - tighten backoff jitter upper bound\n- require absolute HTTPS nextLink\n- return explicit oversized page response error\n- align OnBlocked docs with implemented actions\n\nCo-authored-by: Copilot <[email protected]> * chore: retrigger CI for PR Azure#6954 Co-authored-by: Copilot <[email protected]> * chore: retrigger CI for transient external failures Co-authored-by: Copilot <[email protected]> * fix(azdext): address actionable main PR review items - remove mutable redirect lookup test hook via injected helper - document scope detector servicebus ambiguity and ACR scope semantics - use slices.Sort for deterministic custom rule ordering - clarify TokenProvider usage guidance Co-authored-by: Copilot <[email protected]> * fix(azdext): address remaining maintainer review items Co-authored-by: Copilot <[email protected]> * chore(agents): remove unrelated whitespace-only changes Co-authored-by: Copilot <[email protected]> * fix(azdext): redact blocked URL details in policy callback path Co-authored-by: Copilot <[email protected]> * fix(azdext): add x-ms-client-request-id and align resilient headers Co-authored-by: Copilot <[email protected]> * fix: address PR review feedback from wbreza - Prepend custom scope rules before defaults so overrides work (#1) - Redact URL query params in ScopeDetectorError to prevent leaking secrets (#2) - Add versioned User-Agent string, make configurable via ResilientClientOptions (#3) - Set done=true on Collect truncation to prevent surprise continuation (#4) - Add azdext SDK version constant (version.go) Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
Owner
Author
|
Consolidated into single PR: Azure#7025 |
jongio
added a commit
that referenced
this pull request
Mar 13, 2026
- Use merged_at instead of merged for reliable merge detection (thread #1) - Expand isDocOnlyPr to handle doc-adjacent assets (thread #2) - Replace N+1 API calls with git.getTree for doc inventory (thread #3) - Fix README trigger types to match actual workflow config (thread #5) Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Mar 13, 2026
- Pin actions to commit SHAs (actions/checkout, azure/login) - Cap all_open/list mode to MAX_PRS_PER_RUN=20 - Cap AI output: MAX_REASON_LENGTH=200, MAX_SUMMARY_LENGTH=500 - Add MAX_IMPACTS=15 to limit AI-generated impact count - Add MAX_CONTENT_SIZE_BYTES=50KB per doc file - Sanitize doc manifest content (titles, topics, headings) - Reject unknown repos from AI output (not just warn) - Validate repo format with regex (owner/repo) - Block path traversal in AI-returned paths - Sanitize PR title in log output (strip control chars) - Strip HTML from existing PR body in closeCompanionPrs - Remove error messages from tracking comment (prevent data leak) - Upper-bound PR number input to 999999 - Rename TRUSTED_DOC_INVENTORY to DOC_INVENTORY tag Red team findings addressed: #2, #5, Azure#6, Azure#8, Azure#9, Azure#10, Azure#11 Admin items remaining: #1 (env gating), #3 (token scope), #4 (OIDC vars) Co-authored-by: Copilot <[email protected]>
jongio
pushed a commit
that referenced
this pull request
Mar 25, 2026
* Add auth pre-flight validation for agents (Azure#7234) Add --check flag to 'azd auth token' for lightweight auth validation. Agents can call 'azd auth token --check' to validate authentication state with exit code 0 (valid) or non-zero (invalid) without producing standard output. This prevents costly retry loops where agents speculatively call auth token and parse errors. Enhance 'azd auth status --output json' to include expiresOn field, giving agents machine-readable token expiry information for proactive re-authentication. Improve LoginGuardMiddleware to wrap ErrNoCurrentUser with actionable ErrorWithSuggestion guidance, while preserving original error types for cancellations and transient failures. Changes: - cmd/auth_token.go: Add --check flag with early-exit validation - cmd/auth_token_test.go: Add 3 test cases (check success/failure/not-logged-in) - cmd/auth_status.go: Populate ExpiresOn from token validation - pkg/contracts/auth.go: Add ExpiresOn field to StatusResult - cmd/middleware/login_guard.go: Wrap ErrNoCurrentUser with suggestion Fixes Azure#7234 Co-authored-by: Copilot <[email protected]> * Address review feedback: remove redundant branches, add expiresOn tests - Remove redundant 'if a.flags.check' branches in auth_token.go that duplicated the same return (Copilot review comment #2) - Add StatusResult JSON serialization tests verifying expiresOn is present when authenticated and omitted when unauthenticated (Copilot review comment #3) Co-authored-by: Copilot <[email protected]> * Refactor: replace auth token --check with auth status exit code (Azure#7234) Instead of adding a --check flag to the hidden 'auth token' command, make the existing 'auth status --output json' command agent-friendly: - Exit non-zero when unauthenticated in machine-readable mode, so agents can rely on exit code without parsing output - expiresOn field already added to StatusResult in this PR - Remove --check flag and its tests (net -90 lines) Agents can now validate auth with: azd auth status --output json # exit 0 + JSON with expiresOn = valid # exit 1 + JSON with status:unauthenticated = invalid This is more discoverable than a hidden flag on a hidden command. Co-authored-by: Copilot <[email protected]> * Remove tmp/ from tracking, add to .gitignore Co-authored-by: Copilot <[email protected]> * Revert "Remove tmp/ from tracking, add to .gitignore" This reverts commit 7253f21. * Remove tmp/ files from PR (not part of Azure#7234) Co-authored-by: Copilot <[email protected]> * Address review: exit non-zero in both modes, fix double output Per @JeffreyCA feedback: - Return auth.ErrNoCurrentUser when unauthenticated in both JSON and interactive modes (exit non-zero in all cases) - In JSON mode, format output before returning error to avoid double-print - In interactive mode, show status UX then exit non-zero Co-authored-by: Copilot <[email protected]> * Revert non-zero exit for unauthenticated status Per @vhvb1989 feedback: unauthenticated is a valid result, not a command failure. Non-zero exit should only be for unexpected errors. The expiresOn and LoginGuardMiddleware improvements remain. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
jongio
pushed a commit
that referenced
this pull request
Mar 27, 2026
* Add auth pre-flight validation for agents (Azure#7234) Add --check flag to 'azd auth token' for lightweight auth validation. Agents can call 'azd auth token --check' to validate authentication state with exit code 0 (valid) or non-zero (invalid) without producing standard output. This prevents costly retry loops where agents speculatively call auth token and parse errors. Enhance 'azd auth status --output json' to include expiresOn field, giving agents machine-readable token expiry information for proactive re-authentication. Improve LoginGuardMiddleware to wrap ErrNoCurrentUser with actionable ErrorWithSuggestion guidance, while preserving original error types for cancellations and transient failures. Changes: - cmd/auth_token.go: Add --check flag with early-exit validation - cmd/auth_token_test.go: Add 3 test cases (check success/failure/not-logged-in) - cmd/auth_status.go: Populate ExpiresOn from token validation - pkg/contracts/auth.go: Add ExpiresOn field to StatusResult - cmd/middleware/login_guard.go: Wrap ErrNoCurrentUser with suggestion Fixes Azure#7234 Co-authored-by: Copilot <[email protected]> * Address review feedback: remove redundant branches, add expiresOn tests - Remove redundant 'if a.flags.check' branches in auth_token.go that duplicated the same return (Copilot review comment #2) - Add StatusResult JSON serialization tests verifying expiresOn is present when authenticated and omitted when unauthenticated (Copilot review comment #3) Co-authored-by: Copilot <[email protected]> * Refactor: replace auth token --check with auth status exit code (Azure#7234) Instead of adding a --check flag to the hidden 'auth token' command, make the existing 'auth status --output json' command agent-friendly: - Exit non-zero when unauthenticated in machine-readable mode, so agents can rely on exit code without parsing output - expiresOn field already added to StatusResult in this PR - Remove --check flag and its tests (net -90 lines) Agents can now validate auth with: azd auth status --output json # exit 0 + JSON with expiresOn = valid # exit 1 + JSON with status:unauthenticated = invalid This is more discoverable than a hidden flag on a hidden command. Co-authored-by: Copilot <[email protected]> * Remove tmp/ from tracking, add to .gitignore Co-authored-by: Copilot <[email protected]> * Revert "Remove tmp/ from tracking, add to .gitignore" This reverts commit 7253f21. * Remove tmp/ files from PR (not part of Azure#7234) Co-authored-by: Copilot <[email protected]> * Address review: exit non-zero in both modes, fix double output Per @JeffreyCA feedback: - Return auth.ErrNoCurrentUser when unauthenticated in both JSON and interactive modes (exit non-zero in all cases) - In JSON mode, format output before returning error to avoid double-print - In interactive mode, show status UX then exit non-zero Co-authored-by: Copilot <[email protected]> * Revert non-zero exit for unauthenticated status Per @vhvb1989 feedback: unauthenticated is a valid result, not a command failure. Non-zero exit should only be for unexpected errors. The expiresOn and LoginGuardMiddleware improvements remain. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
jongio
pushed a commit
that referenced
this pull request
Mar 27, 2026
* Add auth pre-flight validation for agents (Azure#7234) Add --check flag to 'azd auth token' for lightweight auth validation. Agents can call 'azd auth token --check' to validate authentication state with exit code 0 (valid) or non-zero (invalid) without producing standard output. This prevents costly retry loops where agents speculatively call auth token and parse errors. Enhance 'azd auth status --output json' to include expiresOn field, giving agents machine-readable token expiry information for proactive re-authentication. Improve LoginGuardMiddleware to wrap ErrNoCurrentUser with actionable ErrorWithSuggestion guidance, while preserving original error types for cancellations and transient failures. Changes: - cmd/auth_token.go: Add --check flag with early-exit validation - cmd/auth_token_test.go: Add 3 test cases (check success/failure/not-logged-in) - cmd/auth_status.go: Populate ExpiresOn from token validation - pkg/contracts/auth.go: Add ExpiresOn field to StatusResult - cmd/middleware/login_guard.go: Wrap ErrNoCurrentUser with suggestion Fixes Azure#7234 Co-authored-by: Copilot <[email protected]> * Address review feedback: remove redundant branches, add expiresOn tests - Remove redundant 'if a.flags.check' branches in auth_token.go that duplicated the same return (Copilot review comment #2) - Add StatusResult JSON serialization tests verifying expiresOn is present when authenticated and omitted when unauthenticated (Copilot review comment #3) Co-authored-by: Copilot <[email protected]> * Refactor: replace auth token --check with auth status exit code (Azure#7234) Instead of adding a --check flag to the hidden 'auth token' command, make the existing 'auth status --output json' command agent-friendly: - Exit non-zero when unauthenticated in machine-readable mode, so agents can rely on exit code without parsing output - expiresOn field already added to StatusResult in this PR - Remove --check flag and its tests (net -90 lines) Agents can now validate auth with: azd auth status --output json # exit 0 + JSON with expiresOn = valid # exit 1 + JSON with status:unauthenticated = invalid This is more discoverable than a hidden flag on a hidden command. Co-authored-by: Copilot <[email protected]> * Remove tmp/ from tracking, add to .gitignore Co-authored-by: Copilot <[email protected]> * Revert "Remove tmp/ from tracking, add to .gitignore" This reverts commit 7253f21. * Remove tmp/ files from PR (not part of Azure#7234) Co-authored-by: Copilot <[email protected]> * Address review: exit non-zero in both modes, fix double output Per @JeffreyCA feedback: - Return auth.ErrNoCurrentUser when unauthenticated in both JSON and interactive modes (exit non-zero in all cases) - In JSON mode, format output before returning error to avoid double-print - In interactive mode, show status UX then exit non-zero Co-authored-by: Copilot <[email protected]> * Revert non-zero exit for unauthenticated status Per @vhvb1989 feedback: unauthenticated is a valid result, not a command failure. Non-zero exit should only be for unexpected errors. The expiresOn and LoginGuardMiddleware improvements remain. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
jongio
pushed a commit
that referenced
this pull request
Mar 27, 2026
* Add auth pre-flight validation for agents (Azure#7234) Add --check flag to 'azd auth token' for lightweight auth validation. Agents can call 'azd auth token --check' to validate authentication state with exit code 0 (valid) or non-zero (invalid) without producing standard output. This prevents costly retry loops where agents speculatively call auth token and parse errors. Enhance 'azd auth status --output json' to include expiresOn field, giving agents machine-readable token expiry information for proactive re-authentication. Improve LoginGuardMiddleware to wrap ErrNoCurrentUser with actionable ErrorWithSuggestion guidance, while preserving original error types for cancellations and transient failures. Changes: - cmd/auth_token.go: Add --check flag with early-exit validation - cmd/auth_token_test.go: Add 3 test cases (check success/failure/not-logged-in) - cmd/auth_status.go: Populate ExpiresOn from token validation - pkg/contracts/auth.go: Add ExpiresOn field to StatusResult - cmd/middleware/login_guard.go: Wrap ErrNoCurrentUser with suggestion Fixes Azure#7234 Co-authored-by: Copilot <[email protected]> * Address review feedback: remove redundant branches, add expiresOn tests - Remove redundant 'if a.flags.check' branches in auth_token.go that duplicated the same return (Copilot review comment #2) - Add StatusResult JSON serialization tests verifying expiresOn is present when authenticated and omitted when unauthenticated (Copilot review comment #3) Co-authored-by: Copilot <[email protected]> * Refactor: replace auth token --check with auth status exit code (Azure#7234) Instead of adding a --check flag to the hidden 'auth token' command, make the existing 'auth status --output json' command agent-friendly: - Exit non-zero when unauthenticated in machine-readable mode, so agents can rely on exit code without parsing output - expiresOn field already added to StatusResult in this PR - Remove --check flag and its tests (net -90 lines) Agents can now validate auth with: azd auth status --output json # exit 0 + JSON with expiresOn = valid # exit 1 + JSON with status:unauthenticated = invalid This is more discoverable than a hidden flag on a hidden command. Co-authored-by: Copilot <[email protected]> * Remove tmp/ from tracking, add to .gitignore Co-authored-by: Copilot <[email protected]> * Revert "Remove tmp/ from tracking, add to .gitignore" This reverts commit 7253f21. * Remove tmp/ files from PR (not part of Azure#7234) Co-authored-by: Copilot <[email protected]> * Address review: exit non-zero in both modes, fix double output Per @JeffreyCA feedback: - Return auth.ErrNoCurrentUser when unauthenticated in both JSON and interactive modes (exit non-zero in all cases) - In JSON mode, format output before returning error to avoid double-print - In interactive mode, show status UX then exit non-zero Co-authored-by: Copilot <[email protected]> * Revert non-zero exit for unauthenticated status Per @vhvb1989 feedback: unauthenticated is a valid result, not a command failure. Non-zero exit should only be for unexpected errors. The expiresOn and LoginGuardMiddleware improvements remain. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
jongio
pushed a commit
that referenced
this pull request
Mar 27, 2026
* Add auth pre-flight validation for agents (Azure#7234) Add --check flag to 'azd auth token' for lightweight auth validation. Agents can call 'azd auth token --check' to validate authentication state with exit code 0 (valid) or non-zero (invalid) without producing standard output. This prevents costly retry loops where agents speculatively call auth token and parse errors. Enhance 'azd auth status --output json' to include expiresOn field, giving agents machine-readable token expiry information for proactive re-authentication. Improve LoginGuardMiddleware to wrap ErrNoCurrentUser with actionable ErrorWithSuggestion guidance, while preserving original error types for cancellations and transient failures. Changes: - cmd/auth_token.go: Add --check flag with early-exit validation - cmd/auth_token_test.go: Add 3 test cases (check success/failure/not-logged-in) - cmd/auth_status.go: Populate ExpiresOn from token validation - pkg/contracts/auth.go: Add ExpiresOn field to StatusResult - cmd/middleware/login_guard.go: Wrap ErrNoCurrentUser with suggestion Fixes Azure#7234 Co-authored-by: Copilot <[email protected]> * Address review feedback: remove redundant branches, add expiresOn tests - Remove redundant 'if a.flags.check' branches in auth_token.go that duplicated the same return (Copilot review comment #2) - Add StatusResult JSON serialization tests verifying expiresOn is present when authenticated and omitted when unauthenticated (Copilot review comment #3) Co-authored-by: Copilot <[email protected]> * Refactor: replace auth token --check with auth status exit code (Azure#7234) Instead of adding a --check flag to the hidden 'auth token' command, make the existing 'auth status --output json' command agent-friendly: - Exit non-zero when unauthenticated in machine-readable mode, so agents can rely on exit code without parsing output - expiresOn field already added to StatusResult in this PR - Remove --check flag and its tests (net -90 lines) Agents can now validate auth with: azd auth status --output json # exit 0 + JSON with expiresOn = valid # exit 1 + JSON with status:unauthenticated = invalid This is more discoverable than a hidden flag on a hidden command. Co-authored-by: Copilot <[email protected]> * Remove tmp/ from tracking, add to .gitignore Co-authored-by: Copilot <[email protected]> * Revert "Remove tmp/ from tracking, add to .gitignore" This reverts commit 7253f21. * Remove tmp/ files from PR (not part of Azure#7234) Co-authored-by: Copilot <[email protected]> * Address review: exit non-zero in both modes, fix double output Per @JeffreyCA feedback: - Return auth.ErrNoCurrentUser when unauthenticated in both JSON and interactive modes (exit non-zero in all cases) - In JSON mode, format output before returning error to avoid double-print - In interactive mode, show status UX then exit non-zero Co-authored-by: Copilot <[email protected]> * Revert non-zero exit for unauthenticated status Per @vhvb1989 feedback: unauthenticated is a valid result, not a command failure. Non-zero exit should only be for unexpected errors. The expiresOn and LoginGuardMiddleware improvements remain. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
jongio
pushed a commit
that referenced
this pull request
Mar 27, 2026
* Add auth pre-flight validation for agents (Azure#7234) Add --check flag to 'azd auth token' for lightweight auth validation. Agents can call 'azd auth token --check' to validate authentication state with exit code 0 (valid) or non-zero (invalid) without producing standard output. This prevents costly retry loops where agents speculatively call auth token and parse errors. Enhance 'azd auth status --output json' to include expiresOn field, giving agents machine-readable token expiry information for proactive re-authentication. Improve LoginGuardMiddleware to wrap ErrNoCurrentUser with actionable ErrorWithSuggestion guidance, while preserving original error types for cancellations and transient failures. Changes: - cmd/auth_token.go: Add --check flag with early-exit validation - cmd/auth_token_test.go: Add 3 test cases (check success/failure/not-logged-in) - cmd/auth_status.go: Populate ExpiresOn from token validation - pkg/contracts/auth.go: Add ExpiresOn field to StatusResult - cmd/middleware/login_guard.go: Wrap ErrNoCurrentUser with suggestion Fixes Azure#7234 Co-authored-by: Copilot <[email protected]> * Address review feedback: remove redundant branches, add expiresOn tests - Remove redundant 'if a.flags.check' branches in auth_token.go that duplicated the same return (Copilot review comment #2) - Add StatusResult JSON serialization tests verifying expiresOn is present when authenticated and omitted when unauthenticated (Copilot review comment #3) Co-authored-by: Copilot <[email protected]> * Refactor: replace auth token --check with auth status exit code (Azure#7234) Instead of adding a --check flag to the hidden 'auth token' command, make the existing 'auth status --output json' command agent-friendly: - Exit non-zero when unauthenticated in machine-readable mode, so agents can rely on exit code without parsing output - expiresOn field already added to StatusResult in this PR - Remove --check flag and its tests (net -90 lines) Agents can now validate auth with: azd auth status --output json # exit 0 + JSON with expiresOn = valid # exit 1 + JSON with status:unauthenticated = invalid This is more discoverable than a hidden flag on a hidden command. Co-authored-by: Copilot <[email protected]> * Remove tmp/ from tracking, add to .gitignore Co-authored-by: Copilot <[email protected]> * Revert "Remove tmp/ from tracking, add to .gitignore" This reverts commit 7253f21. * Remove tmp/ files from PR (not part of Azure#7234) Co-authored-by: Copilot <[email protected]> * Address review: exit non-zero in both modes, fix double output Per @JeffreyCA feedback: - Return auth.ErrNoCurrentUser when unauthenticated in both JSON and interactive modes (exit non-zero in all cases) - In JSON mode, format output before returning error to avoid double-print - In interactive mode, show status UX then exit non-zero Co-authored-by: Copilot <[email protected]> * Revert non-zero exit for unauthenticated status Per @vhvb1989 feedback: unauthenticated is a valid result, not a command failure. Non-zero exit should only be for unexpected errors. The expiresOn and LoginGuardMiddleware improvements remain. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Apr 3, 2026
…er workflow (Azure#7424) * Improve code coverage pipeline: Phase 1 tests, coverage tooling, and developer workflow - Add 16 new test files for cmd/, middleware/, mcp/, and github packages - Create local coverage orchestrator (Get-LocalCoverageReport.ps1) with -UnitOnly, -MergeWithCI hybrid mode, -Html report options - Add generated code filter (Filter-GeneratedCoverage.ps1) for *.pb.go - Integrate filter into CI pipeline (code-coverage-upload.yml) - Raise CI coverage gate from 52% to 55% (release-cli.yml) - Add Code Coverage section to CONTRIBUTING.md with 4-mode matrix - Create cli/azd/docs/code-coverage-guide.md deep-dive reference - Cross-reference all 4 coverage scripts to each other and guide Co-authored-by: Copilot <[email protected]> * Phase 2: Add coverage tests for auth, keyvault, pipeline, cosmosdb, sqldb, armmsi - pkg/auth: 7 new test files, 54.2% -> 80.0% (+25.8pp) Covers CredentialForCurrentUser branches, loadClaims/saveClaims, Mode/SetBuiltInAuthMode, Logout cleanup, RemoteCredential, caches - pkg/keyvault: ParseKeyVaultAppReference edge cases, validation, 37.8% -> 41.5% - pkg/pipeline: Template rendering, provider lifecycle, pure logic, 27.8% -> 30.1% - pkg/cosmosdb: New test file from 0% -> 86.7% (mock ARM client) - pkg/sqldb: New test file from 0% -> 94.4% (mock ARM client) - pkg/armmsi: New test file from 0% -> 76.7% (mock ARM client) Co-authored-by: Copilot <[email protected]> * docs: fill documentation gaps for coverage scripts and guide - Add missing .PARAMETER PipelineDefinitionId to Get-CICoverageReport.ps1 help - Add working directory note to CONTRIBUTING.md Code Coverage section - Add Scripts Reference table to code-coverage-guide.md listing all 7 scripts Co-authored-by: Copilot <[email protected]> * fix: resolve preflight issues (gofmt, unused types, line length) - Run gofmt -s on all new test files - Remove unused fakeHTTPClient from credential_providers_coverage_test.go - Remove unused jwtTokenCredential from wave3_coverage_test.go - Break long line in middleware_coverage2_test.go (lll) - Fix trailing whitespace in pipeline_coverage_test.go Co-authored-by: Copilot <[email protected]> * feat: add mage coverage targets and address review feedback - Add Coverage namespace to magefile with 6 targets (unit, full, hybrid, ci, html, check) - Update CONTRIBUTING.md and code-coverage-guide.md with mage target docs - Fix stderr redirect in Get-LocalCoverageReport.ps1 token acquisition (review thread #1) - Fix tautological sentinel test in keyvault_coverage_test.go (review thread #3) Co-authored-by: Copilot <[email protected]> * fix: resolve CI failures - cspell words and gosec nolint for path traversal Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/75a62821-a5d2-4f5b-8372-b3b8fe8d1802 Co-authored-by: jongio <[email protected]> * fix: CI test failures — env var isolation and CI skip - middleware_coverage2_test: skip TestLoginGuard_EnsureLogin_ConfirmError in CI where IsRunningOnCI() short-circuits before console Confirm - manager_coverage_test: use t.Setenv+os.Unsetenv for proper env var isolation (code uses os.LookupEnv which distinguishes empty vs unset) - Remove bare os.Unsetenv calls that mutated shared process state Co-authored-by: Copilot <[email protected]> * fix: lower coverage:check threshold to 50% for unit-only mode The coverage:check mage target uses unit-only mode (52.7%) but was defaulting to 55% threshold (designed for CI combined unit+integration). Lower to 50% which is appropriate for unit-only, and update docs to clarify the distinction. Co-authored-by: Copilot <[email protected]> * fix: rebase onto main, update tests for fixableError API change - Rebase onto latest main to pick up error.go refactor (PR Azure#7401) - Replace classifyError/ErrorCategory tests with fixableError tests - Fix ExtensionRunError struct fields (ExitCode -> ExtensionId+Err) - Add COVERAGE_MIN override tip to CONTRIBUTING.md (reviewer feedback) Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]>
jongio
added a commit
that referenced
this pull request
Apr 6, 2026
- Route targetResource==nil through event-aware postprovisionK8sError so predeploy doesn't silently skip (thread #1, High) - Add preDeployEvent constant replacing raw "predeploy" strings for consistency with postProvisionEvent (thread #2) - Add console message assertions to graceful-skip tests verifying the warning was actually emitted (thread #3) - Add predeploy failure tests for credential and namespace error paths confirming errors propagate for non-postprovision events (thread #4) - Refactor createAksServiceTarget to delegate to createAksServiceTargetWithResourceManager, eliminating ~40 lines of duplication (thread #5) - Add structured telemetry span (AksPostprovisionSkipEvent) to the graceful-skip path for production monitoring (thread Azure#6) Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Apr 6, 2026
- Route targetResource==nil through event-aware postprovisionK8sError so predeploy doesn't silently skip (thread #1, High) - Add preDeployEvent constant replacing raw "predeploy" strings for consistency with postProvisionEvent (thread #2) - Add console message assertions to graceful-skip tests verifying the warning was actually emitted (thread #3) - Add predeploy failure tests for credential and namespace error paths confirming errors propagate for non-postprovision events (thread #4) - Refactor createAksServiceTarget to delegate to createAksServiceTargetWithResourceManager, eliminating ~40 lines of duplication (thread #5) - Add structured telemetry span (AksPostprovisionSkipEvent) to the graceful-skip path for production monitoring (thread Azure#6) Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Apr 7, 2026
- Route targetResource==nil through event-aware postprovisionK8sError so predeploy doesn't silently skip (thread #1, High) - Add preDeployEvent constant replacing raw "predeploy" strings for consistency with postProvisionEvent (thread #2) - Add console message assertions to graceful-skip tests verifying the warning was actually emitted (thread #3) - Add predeploy failure tests for credential and namespace error paths confirming errors propagate for non-postprovision events (thread #4) - Refactor createAksServiceTarget to delegate to createAksServiceTargetWithResourceManager, eliminating ~40 lines of duplication (thread #5) - Add structured telemetry span (AksPostprovisionSkipEvent) to the graceful-skip path for production monitoring (thread Azure#6) Co-authored-by: Copilot <[email protected]>
jongio
added a commit
that referenced
this pull request
Apr 9, 2026
…zure#7501) * fix: gracefully handle missing AKS cluster during postprovision hook When infrastructure doesn't include AKS resources, the postprovision lifecycle hook in the AKS service target fails fatally trying to set up the Kubernetes context. This is expected in multi-phase provisioning workflows where AKS gets provisioned later. Modified setK8sContext() to detect the postprovision event and skip gracefully (with a user-visible warning) instead of failing when: - GetTargetResource returns an error (resource not found) - Target resource has an empty name (SupportsDelayedProvisioning) - Cluster credentials are unavailable (ensureClusterContext fails) - Namespace creation fails (ensureNamespace fails) The predeploy event path is unchanged and still fails fatally, ensuring deployment-time errors are not masked. Extracted skipPostprovisionK8sSetup() helper to eliminate repeated warning/log/return-nil pattern across all four skip points. Fixes Azure#3272 Co-authored-by: Copilot <[email protected]> * address review: propagate ctx cancellation, add namespace failure test - skipPostprovisionK8sSetup now checks ctx.Err() before returning nil to avoid swallowing context cancellation/timeouts (Ctrl+C) - Added Test_Postprovision_Skips_When_Namespace_Fails covering the ensureNamespace failure path during postprovision Co-authored-by: Copilot <[email protected]> * refactor: address MQ review — typed constant, DRY helper, ctx cancellation test - Replace magic string 'postprovision' with typed postProvisionEvent constant - Extract postprovisionK8sError() helper to eliminate 4 identical if-blocks - Add nil-guard on targetResource before calling ResourceName() - Fix log.Printf double newline and redundant .Error() call - Add Test_Postprovision_Propagates_Context_Cancellation to cover the ctx.Err() safety valve in skipPostprovisionK8sSetup Co-authored-by: Copilot <[email protected]> * fix: address review feedback — event guards, constants, test assertions - Route targetResource==nil through event-aware postprovisionK8sError so predeploy doesn't silently skip (thread #1, High) - Add preDeployEvent constant replacing raw "predeploy" strings for consistency with postProvisionEvent (thread #2) - Add console message assertions to graceful-skip tests verifying the warning was actually emitted (thread #3) - Add predeploy failure tests for credential and namespace error paths confirming errors propagate for non-postprovision events (thread #4) - Refactor createAksServiceTarget to delegate to createAksServiceTargetWithResourceManager, eliminating ~40 lines of duplication (thread #5) - Add structured telemetry span (AksPostprovisionSkipEvent) to the graceful-skip path for production monitoring (thread Azure#6) Co-authored-by: Copilot <[email protected]> * fix: address re-review - telemetry span, DRY assertions, ErrorContains - Use span.End() with skip.reason attribute instead of EndWithStatus() to avoid marking intentional skips as errors in telemetry dashboards - Extract assertSkipWarningEmitted helper to deduplicate console assertion blocks across two test functions - Add ErrorContains assertion in Test_Predeploy_Fails_When_Credentials_Fail for consistency with sibling namespace test Co-authored-by: Copilot <[email protected]> * fix: narrow postprovision graceful skip to not-found only Address @weikanglim's feedback: limit the graceful skip path to the explicit delayed-provisioning case (empty ResourceName) only. GetTargetResource, ensureClusterContext, and ensureNamespace errors now propagate during postprovision — real failures are no longer masked. Removed postprovisionK8sError helper. Fixed gofmt import ordering. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Links
Stack position
Stack / Merge Plan (Uber Plan)
This PR is Step 4 of 6 in the full rollout.
Required merge order
How to land this safely