Skip to content

Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot)#38152

Merged
pelikhan merged 8 commits into
mainfrom
copilot/fix-checkout-pr-branch
Jun 9, 2026
Merged

Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot)#38152
pelikhan merged 8 commits into
mainfrom
copilot/fix-checkout-pr-branch

Conversation

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

assertTrustedCheckoutRuntime() called getCollaboratorPermissionLevel with context.actor, which returns HTTP 404 for GitHub App actors like "Copilot" since they are not regular users. This caused the checkout step to fail on Copilot-authored PRs.

Changes

  • checkout_pr_branch.cjs
    • Check context.payload.sender?.type before hitting the collaborators API — bypass the check for "Bot" actors in the already-verified non-fork context
    • Wrap the API call in try/catch and disambiguate collaborator API 404 responses:
      • if users.getByUsername also returns 404, treat actor as non-user app/bot and allow checkout
      • if users.getByUsername succeeds, fail checkout as non-collaborator
      • non-404 errors still propagate
const senderType = context.payload.sender?.type;
if (senderType === "Bot") {
  core.info(`Runtime safety check passed for bot/app actor '${actor}' (sender type: ${senderType})`);
  return;
}

try {
  // ... getCollaboratorPermissionLevel call
} catch (err) {
  if (err.status === 404) {
    try {
      await github.rest.users.getByUsername({ username: actor });
      throw new Error(`Refusing PR checkout: actor '${actor}' is not a collaborator (requires write or higher)`);
    } catch (userErr) {
      if (userErr.status === 404) {
        core.info(`Runtime safety check passed for app actor '${actor}' (not a regular user)`);
        return;
      }
      throw userErr;
    }
  }
  throw err;
}
  • checkout_pr_branch.test.cjs — adds/updates cases for Bot bypass, 404 app/bot fallback with user lookup disambiguation, non-collaborator user failure, and non-404 error propagation
  • Reverted unrelated workflow lockfile churn so the PR scope remains focused on checkout runtime safety

Copilot AI and others added 2 commits June 9, 2026 16:22
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
…dabot)

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix checkout_pr_branch.cjs for Copilot-authored PRs Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot) Jun 9, 2026
Copilot finished work on behalf of lpcox June 9, 2026 16:29
Copilot AI requested a review from lpcox June 9, 2026 16:29
@pelikhan pelikhan marked this pull request as ready for review June 9, 2026 16:32
Copilot AI review requested due to automatic review settings June 9, 2026 16:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes assertTrustedCheckoutRuntime() failures for GitHub App / bot actors (e.g., Copilot) by avoiding or recovering from collaborator-permission lookups that can 404 for non-user actors, and adds tests to cover the new behavior. Also includes a broad regeneration of workflow lockfiles that tightens create_issue.body validation.

Changes:

  • Bypass collaborator permission checks for sender.type of Bot and Mannequin in assertTrustedCheckoutRuntime().
  • Add try/catch around collaborator permission lookup and handle 404s as a fallback path.
  • Regenerate workflow *.lock.yml files to add minLength for create_issue.body validation.
Show a summary per file
File Description
actions/setup/js/checkout_pr_branch.cjs Adjust runtime trust checks to handle bot/app actors and 404s from collaborator permission lookups.
actions/setup/js/checkout_pr_branch.test.cjs Add tests for Bot/Mannequin bypass, 404 fallback, and non-404 error propagation.
.github/workflows/workflow-skill-extractor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/workflow-normalizer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/workflow-health-manager.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/video-analyzer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/test-workflow.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/super-linter.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/step-name-alignment.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/static-analysis-report.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/stale-repo-identifier.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/spec-librarian.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-workflow-call-with-inputs.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-update-cross-repo-pr.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-temporary-id.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-project.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-pi.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-otel-backends.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-opencode.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-gemini.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-crush.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-create-cross-repo-pr.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-copilot.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-copilot-sdk.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-copilot-arm.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-copilot-aoai-apikey.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-codex.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-claude.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-ci.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/smoke-antigravity.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/sergo.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/semantic-function-refactor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/security-compliance.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/ruflo-backed-task.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/refactoring-cadence.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/pr-triage-agent.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/poem-bot.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/plan.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/outcome-collector.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/otlp-data-quality-validator.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/objective-impact-report.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/metrics-collector.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/lint-monster.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/issue-arborist.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/hippo-embed.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/gpclean.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/go-pattern-detector.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/go-fan.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/firewall.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/example-permissions-warning.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/duplicate-code-detector.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/discussion-task-miner.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/dev.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/designer-drift-audit.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/deployment-incident-monitor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/dependabot-go-checker.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/dependabot-burner.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/delight.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/deep-report.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-windows-terminal-integration-builder.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-token-consumption-report.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-testify-uber-super-expert.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-team-status.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-syntax-error-quality.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-spdd-spec-planner.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-skill-optimizer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-security-red-team.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-safeoutputs-git-simulator.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-safe-outputs-conformance.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-safe-output-optimizer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-reliability-review.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-otel-instrumentation-advisor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-multi-device-docs-tester.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-model-inventory.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-mcp-concurrency-analysis.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-max-ai-credits-test.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-grafana-otel-instrumentation-advisor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-function-namer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-formal-spec-verifier.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-file-diet.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-doc-healer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-credit-limit-test.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-community-attribution.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-cli-tools-tester.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-cli-performance.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-cache-strategy-analyzer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-byok-ollama-test.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-awf-spec-compiler-surfacing.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-aw-cross-repo-compile-check.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-architecture-diagram.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-ambient-context-optimizer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/daily-agentrx-trace-optimizer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/copilot-opt.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/contribution-check.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/codex-github-remote-mcp-test.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/cli-version-checker.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/cli-consistency-checker.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/ci-doctor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/breaking-change-checker.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/bot-detection.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/aw-failure-investigator.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/architecture-guardian.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/agentic-token-trend-audit.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/agentic-token-optimizer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/agentic-token-audit.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/agent-persona-explorer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/agent-performance-analyzer.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/ace-editor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.
.github/workflows/ab-testing-advisor.lock.yml Lockfile regen: adds minLength constraint for create_issue.body.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 109/109 changed files
  • Comments generated: 2

Comment on lines +160 to +164
if (err.status === 404) {
core.info(`Runtime safety check passed for app actor '${actor}' (not a regular user)`);
return;
}
throw err;
Comment on lines 561 to +563
"sanitize": true,
"maxLength": 65000
"maxLength": 65000,
"minLength": 20
@lpcox

lpcox commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@copilot review all comments and address unresolved review feedback.
Please summarize any remaining blockers after that.

Generated by 👨‍🍳 PR Sous Chef · 71.4 AIC · ⌖ 1 AIC · ⊞ 17.2K ·

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #38152 does not have the "implementation" label and has 0 new lines of code in business logic directories (threshold is 100).

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
@lpcox lpcox added the smoke label Jun 9, 2026
@github-actions github-actions Bot mentioned this pull request Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Smoke Antigravity MISSION COMPLETE! Antigravity has spoken. ✨

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Smoke Pi MISSION COMPLETE! Pi delivered. 🥧

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed review feedback in commit fa841f0.

  • Tightened 404 handling in assertTrustedCheckoutRuntime() by checking users.getByUsername before trusting 404, so non-collaborator real users now fail.
  • Added test coverage for the non-collaborator 404 case.
  • Reverted unrelated workflow lockfile regeneration changes from this PR.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot - AOAI (apikey) is now investigating this pull request. Sources say the story is developing...

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100 — Excellent

Analyzed 4 test(s) in 1 JavaScript file: 4 design tests (behavioral contracts), 0 implementation tests, 0 guideline violations.

📊 Metrics & Test Classification (4 tests analyzed)
Metric Value
New/modified tests analyzed 4
✅ Design tests (behavioral contracts) 4 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 4 (100%)
Duplicate test clusters 0
Test inflation detected No (ratio 1.5:1 — 48 test lines / 32 production lines)
🚨 Coding-guideline violations 0

Test Classification Details

Test File Classification Issues Detected
should allow checkout for Bot actor without calling the collaborator API actions/setup/js/checkout_pr_branch.test.cjs:~261 ✅ Design None
should allow checkout for Mannequin actor without calling the collaborator API actions/setup/js/checkout_pr_branch.test.cjs:~275 ✅ Design Minor: does not assert git exec operations were called
should allow checkout when collaborator API returns 404 (app actor without sender type) actions/setup/js/checkout_pr_branch.test.cjs:~288 ✅ Design None
should fail when collaborator API returns a non-404 error actions/setup/js/checkout_pr_branch.test.cjs:~305 ✅ Design None

Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 4 tests (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). All 4 tests verify behavioral contracts: the bot/Mannequin bypass path, the 404 fallback path, and error propagation for non-404 failures.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · 146 AIC · ⌖ 19.1 AIC ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 4 new tests verify behavioral contracts covering the bot/Mannequin bypass path, the 404 fallback path, and non-404 error propagation.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Bot actor checkout fix

The core fix is sound — bots and app installations fail on the collaborators API with 404, and the dual-strategy (sender type fast-path + 404 catch fallback) correctly handles both the structured case (sender.type available) and the unstructured case (minimal event payloads). The non-fork guard remains the primary security gate. Tests are thorough and cover the new paths well.

Two non-blocking comments posted inline.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer · ⌖ 13.4 AIC

Comment thread actions/setup/js/checkout_pr_branch.cjs Outdated
// Trust them implicitly: the non-fork repository check above already ensures
// the workflow is running in a controlled context.
const senderType = context.payload.sender?.type;
if (senderType === "Bot" || senderType === "Mannequin") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mannequin actor type is semantically wrong here and bypasses permission check without rationale.

Mannequin accounts are placeholder accounts for migrated users (from org/repo imports) — they are neither bots nor app installations. The function comment above explicitly says "Bot and app actors (e.g. Copilot, dependabot[bot])" but silently extends that trust to Mannequins. The log message then misleads operators by printing bot/app actor for a Mannequin.

💡 Why this matters

The collaborators API can resolve Mannequin users — they are registered in GitHub's user system and do have a permission level. So the justification used for bots ("cannot be resolved via the collaborators API") doesn't apply to Mannequins. If a Mannequin-typed event can ever trigger a live workflow (e.g., during GitHub Enterprise import/migration), it bypasses the permission check entirely.

Either:

  1. Remove || senderType === "Mannequin" (let it fall through to the collaborators API check which should work for Mannequins), or
  2. Add a comment explicitly explaining why Mannequin events should be trusted as if they were bots.

The non-fork guard partially mitigates the risk, but the trust model is inconsistent.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot review

Comment thread actions/setup/js/checkout_pr_branch.cjs Outdated
} catch (err) {
// The collaborators API returns 404 for app/bot actors that are not regular
// GitHub users (e.g. when sender.type is unavailable in the event payload).
// Treat these as trusted within the already-verified non-fork context.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch block comment is inaccurate: it fires for more cases than described.

The comment says the API returns 404 "when sender.type is unavailable in the event payload" — but at this point in the function, sender.type has already been checked (line 137). If it was Bot or Mannequin, we already returned. This catch block fires when:

  1. sender.type is absent/undefined — the comment's intended case ✓
  2. sender.type is set to something other than Bot or Mannequin (e.g., "User") but the collaborators API still returns 404 — not covered by the comment
💡 Suggested clarification
} catch (err) {
  // The collaborators API returns 404 for actors that are not regular
  // GitHub users — typically app installations or bots whose sender.type
  // was not Bot/Mannequin (or sender was absent in the event payload).
  // These are trusted within the already-verified non-fork context.
  if (err.status === 404) {

This accurately describes all code paths that reach this catch.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Smoke test 27222112837: FAIL\n#1 ❌ GitHub MCP read\n#2 ✅ Serena\n#3 ✅ Playwright\n#4 ❌ Web-fetch MCP\n#5 ✅ File write\n#6 ✅ Bash verify\n#7 ✅ Build\n#8 ✅ Comment memory\n#9 ✅ Cache memory\n#10 ⏭️ Set issue field

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list 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 oracle has spoken through Smoke Codex · 15.2 AIC · ⌖ 2.35 AIC ·

@pelikhan

pelikhan commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Comment Memory

Steel dawn hums
quiet branches hold the signal
checks bloom, then settle

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list 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 oracle has spoken through Smoke Codex · 15.2 AIC · ⌖ 2.35 AIC ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /diagnose and /tdd — no blocking issues, but a few improvements worth addressing before merge.

📋 Key Themes & Highlights

Key Themes

  • Undocumented security assumption (line 160): the catch (err.status === 404) trust relies on GitHub never returning 404 for real users — true today, but nowhere stated.
  • Test parity gap: the Mannequin bypass test is missing the exec assertions present in the equivalent Bot test.
  • Path-pinning: two tests would benefit from an explicit toHaveBeenCalled assertion to lock in the execution path.

Positive Highlights

  • ✅ Clean two-level strategy: fast-path sender.type bypass + API-404 fallback is a solid defence-in-depth approach
  • ✅ The non-fork check (line 121) provides the outer security boundary, making the inner trust decisions well-bounded
  • ✅ Five meaningful regression tests added covering every new branch
  • ✅ Bot bypass test is thorough — checks API not called and git commands ran

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 322.3 AIC · ⌖ 14 AIC


await runScript();

expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Internal Server Error"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The non-404 error test does not assert that getCollaboratorPermissionLevel was actually called.

The default mock context has no sender, so the code correctly falls into the try/catch — but adding an explicit assertion pins the execution path and guards against a future widening of the fast-path bypass (e.g. additional sender.type values) that would short-circuit before the collaborators API is ever called.

💡 Add
expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).toHaveBeenCalled();


expect(mockGithub.rest.repos.getCollaboratorPermissionLevel).not.toHaveBeenCalled();
expect(mockCore.info).toHaveBeenCalledWith("Runtime safety check passed for bot/app actor 'mannequin-user' (sender type: Mannequin)");
expect(mockCore.setFailed).not.toHaveBeenCalled();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] Missing exec assertions — the Bot test at line 270 explicitly verifies that git fetch and git checkout ran after the bypass; the Mannequin test only checks setFailed was not called.

That passes even if the script bailed out early for an unrelated reason, so the test does not actually prove the checkout happened.

💡 Add these two assertions after line 282
expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "feature-branch", "--depth=2"]);
expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "feature-branch"]);

This brings the Mannequin test to parity with the Bot test.

Comment thread actions/setup/js/checkout_pr_branch.cjs Outdated
// Trust them implicitly: the non-fork repository check above already ensures
// the workflow is running in a controlled context.
const senderType = context.payload.sender?.type;
if (senderType === "Bot" || senderType === "Mannequin") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnose] The Mannequin bypass is included without a comment explaining the concrete trigger scenario.

Mannequin accounts are GitHub Enterprise Server migration placeholders — they are not active GitHub Apps or bots. Under what real workflow event would context.actor be a Mannequin and trigger a PR checkout? If the answer is “it never actually happens but we include it defensively,” that is worth saying explicitly so future reviewers do not have to re-research it.

💡 Suggested comment
// Mannequin accounts are GHES migration placeholders that, like Bots, cannot be
// resolved via the collaborators API. Include them here as a defensive fallback.
if (senderType === "Bot" || senderType === "Mannequin") {

// The collaborators API returns 404 for app/bot actors that are not regular
// GitHub users (e.g. when sender.type is unavailable in the event payload).
// Treat these as trusted within the already-verified non-fork context.
if (err.status === 404) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnose] This catch trusts any 404 from getCollaboratorPermissionLevel, including cases where sender.type is explicitly "User" and the API unexpectedly returns 404.

The security model relies on GitHub never returning 404 for real GitHub users (they receive permission: "none" instead). That assumption holds today but is not stated anywhere — a future API change or an edge case (e.g., a deleted account used as actor) would silently widen the trust surface.

💡 Suggestion: document the API contract assumption
// GitHub’s collaborators API returns 404 only for non-user accounts (e.g. GitHub App
// actors like Copilot). Real GitHub users always receive a permission response, never
// a 404. Trusting 404 here is therefore safe within the already-verified non-fork context.
// Ref: https://docs.github.com/en/rest/collaborators/collaborators#get-repository-permissions-for-a-user
if (err.status === 404) {

With this comment, future maintainers know exactly what assumption to revalidate if the API behaviour changes.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

💥 Smoke Test: Claude — Run 27222113095

Core (#1-12): ✅✅✅✅✅✅✅✅✅✅✅✅
PR Review (#13-19): 13✅ 14✅ 15✅ 16✅ 17✅ 18✅ 19⚠️(skipped)

Overall: PASS ✅ — Claude engine nominal!

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list 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 · 86.4 AIC · ⌖ 30.8 AIC ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💥 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.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list 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 · 86.4 AIC · ⌖ 30.8 AIC

if (senderType === "Bot" || senderType === "Mannequin") {
core.info(`Runtime safety check passed for bot/app actor '${actor}' (sender type: ${senderType})`);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice — bypassing the collaborators API for Bot/Mannequin sender types is a clean guard. Consider a brief comment noting these are non-fork-verified contexts.

});

const permission = permissionData?.permission || "none";
const hasWriteOrHigher = TRUSTED_CHECKOUT_PERMISSIONS.includes(permission);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good defensive use of try/catch around getCollaboratorPermissionLevel — the 404 disambiguation via the users API is a thoughtful touch.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

PR: Fix assertTrustedCheckoutRuntime for bot/app actors (Copilot, dependabot)
1❌ 2✅ 3✅ 4✅ 5✅ 6✅
7✅ 8✅ 9✅ 10❌ 11✅
12✅ 13❌ 14✅ 15✅
Overall: FAIL
Author: @app/copilot-swe-agent | Assignees: @lpcox @Copilot

Warning

Firewall blocked 6 domains

The following domains were blocked by the firewall during workflow execution:

  • accounts.google.com
  • android.clients.google.com
  • clients2.google.com
  • contentautofill.googleapis.com
  • safebrowsingohttpgateway.googleapis.com
  • www.google.com

To allow these domains, add them to the network.allowed list 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 · 191.1 AIC · ⌖ 15.9 AIC ·

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🛠️ Agentic Maintenance updated this pull request branch.

View workflow run

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot run pr-finisher skill

Ran the pr-finisher workflow for this PR. Local validation is green (make fmt, make lint, make test-unit, make test, make recompile), and I reviewed the latest CI snapshot/log access via MCP. No additional code changes were needed beyond d24fe0a; CI on the current head is still action-required/stale and needs a maintainer re-trigger before merge.

Copilot AI requested a review from pelikhan June 9, 2026 17:21
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot run pr-finisher skill

Done. I ran the pr-finisher flow and pushed commit 272779d, which:

  • reverts the unrelated lockfile churn commit,
  • tightens trusted checkout handling by removing the Mannequin bypass,
  • updates tests accordingly.

Local validation passed (make fmt, make lint, make test-unit, make test, make recompile). Prior CI runs on this branch are action_required (no failed jobs/logs available), and CI will need a maintainer re-trigger on the latest head before merge.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan merged commit 04a581e into main Jun 9, 2026
21 checks passed
@pelikhan pelikhan deleted the copilot/fix-checkout-pr-branch branch June 9, 2026 17:44
lpcox added a commit to github/gh-aw-firewall that referenced this pull request Jun 9, 2026
- Resolve merge conflict in smoke-claude.md
- Recompile all 40 workflows with gh-aw v0.79.2
- Run postprocess-smoke-workflows.ts
- Update actions-lock.json to pin gh-aw-actions/setup@v0.79.2

Includes fix for checkout_pr_branch.cjs failing on bot/app actors
(Copilot, dependabot). See github/gh-aw#38134, github/gh-aw#38152.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lpcox added a commit to github/gh-aw-firewall that referenced this pull request Jun 9, 2026
* Upgrade gh-aw to v0.79.2 (pre-release)

- Resolve merge conflict in smoke-claude.md
- Recompile all 40 workflows with gh-aw v0.79.2
- Run postprocess-smoke-workflows.ts
- Update actions-lock.json to pin gh-aw-actions/setup@v0.79.2

Includes fix for checkout_pr_branch.cjs failing on bot/app actors
(Copilot, dependabot). See github/gh-aw#38134, github/gh-aw#38152.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Update CI workflow tests for gh-aw v0.79.2

- smoke-claude: remove --max-turns 2 assertion (no longer emitted as CLI flag)
- security-guard, test-coverage-improver: update setup action SHA/version

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

checkout_pr_branch.cjs fails for Copilot-authored PRs (actor is not a user)

4 participants