Skip to content

ci(miro): regenerate dist on dependabot manifest bumps - #2432

Merged
kyle-sexton merged 2 commits into
mainfrom
cursor/dependabot-miro-bundle-63e1
Aug 12, 2026
Merged

ci(miro): regenerate dist on dependabot manifest bumps#2432
kyle-sexton merged 2 commits into
mainfrom
cursor/dependabot-miro-bundle-63e1

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #2083

Related

N/A

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 55s —— View job


Security Review

  • Fetch PR diff and read changed files
  • Analyze new workflow dependabot-miro-bundle.yml for security issues (triggers, permissions, injection, secrets)
  • Analyze change to ci.yml
  • Report findings

Scope: .github/workflows/ci.yml (+1, adds the new workflow to the schema-validation file list — no security relevance) and the new .github/workflows/dependabot-miro-bundle.yml.

Per instructions, I did not re-report anything zizmor's advisory lane already covers statically (unpinned actions, pull_request_target/dangerous-trigger patterns, excessive permissions:, template injection via ${{ }} in run:). This workflow already does several things right that are worth noting instead of flagging: it uses pull_request (not pull_request_target), it passes github.base_ref through an env: var rather than interpolating it directly into the shell script (avoids script injection), and both third-party actions are pinned to full commit SHAs.


1. Write-scoped, push-capable credential is live in the environment while untrusted dependency code executes

Severity: IMPORTANT | Confidence: Medium

dependabot-miro-bundle.yml L18-26 grants the job contents: write, and actions/checkout (default persist-credentials: true) leaves a push-capable GITHUB_TOKEN credential configured in the runner's git config for the entire job — not just the final push step.

Between that checkout and the intended git push, the job runs npm ci, npm run bundle, npm run verify-bundle against plugins/miro/package.json/package-lock.json — i.e. it executes install-time lifecycle scripts and a custom bundle script pulled from exactly the dependency tree Dependabot just bumped. If a bumped package version is malicious or compromised (a real, recurring npm supply-chain pattern, and precisely the input this workflow is designed to act on), its install/build scripts run with that write-scoped credential already present in the environment. That's a strictly larger blast radius than "regenerate dist/index.min.js" — a malicious script could push to other branches, overwrite unrelated files, or otherwise abuse the token, not just tamper with the intended artifact.

This is a data-flow/trust-boundary concern (privileged credential co-located with untrusted-dependency code execution), distinct from zizmor's static permission/trigger checks.

Suggested mitigation: set persist-credentials: false on the checkout step and authenticate explicitly (e.g. via git remote set-url/GH_TOKEN on the gh/git push command) only in the final "Commit regenerated bundle" step, after npm ci/bundle/verify-bundle have already run. Optionally add npm ci --ignore-scripts if the build doesn't require install scripts.

Fix this →


2. Note (not a finding): Dependabot-triggered pull_request runs typically get a forced read-only GITHUB_TOKEN

Informational

GitHub documents that workflows triggered by dependabot[bot] via the pull_request event normally receive a read-only GITHUB_TOKEN and no secrets regardless of the permissions: block, specifically to prevent the pattern above. If that restriction applies here as expected, the final git push in this workflow will likely fail with a permissions error for genuine Dependabot-triggered runs — a functionality/reliability issue, not a vulnerability, and actually protective. I'm flagging it only because it's directly relevant to finding #1: if this repo/org has any override in place that restores write access for Dependabot actors (e.g. via a custom token or org policy), finding #1 becomes fully live rather than theoretical, so it's worth confirming which case applies before relying on the github.actor == 'dependabot[bot]' gate as a trust boundary.


No other logic, authorization, or injection issues were found in the diff. The github.base_ref/github.head_ref handling and the actor gate are otherwise implemented safely.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c16cf27231

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/dependabot-miro-bundle.yml
@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 52s —— View job


Code Review Complete

  • Gather context (PR diff, CLAUDE.md, REVIEW.md)
  • Understand the request
  • Review changed files
  • Post findings

Summary

Reviewed the two changed files: .github/workflows/ci.yml (adds the new workflow to the allowed-workflows list, correctly alphabetized) and the new .github/workflows/dependabot-miro-bundle.yml. Root CLAUDE.md is empty in this checkout, so no repo-specific guidance applied beyond REVIEW.md's lane split; this repo has a claude-security-review.yml workflow, so security findings are left to that lane per REVIEW.md.

🔴 Important finding posted inline on dependabot-miro-bundle.yml line 62: the workflow triggers on pull_request from dependabot[bot], and GitHub forces GITHUB_TOKEN to read-only with no secrets for Dependabot-triggered pull_request runs — regardless of the permissions: contents: write declared on the job (lines 18-20). That means the final git push step will fail every time, so the bundle is never actually pushed back to the PR branch, and issue #2083 remains unfixed in practice even though CI-allowlisting and script wiring are otherwise correct. .github/dependabot.yml explicitly states this repo avoids a PAT/GitHub App for Dependabot automation, so the fix isn't "add a token secret" — see the inline comment for a suggested split-workflow (workflow_run) pattern that avoids that constraint.

Everything else checked out:

  • npm run bundle / npm run verify-bundle scripts referenced in the workflow exist in plugins/miro/package.json and match.
  • The origin/$BASE_REF...HEAD diff-scoping pattern (with fetch-depth: 0) mirrors the same pattern already used elsewhere in ci.yml, so it's consistent with repo convention.
  • The ci.yml allowlist addition is correctly placed alphabetically and is the only change needed there.

I was not able to trigger an actual Dependabot PR run in this session to empirically confirm the push failure — the finding is based on documented GitHub platform behavior for Dependabot-triggered pull_request events, not something I ran locally.

Comment thread .github/workflows/dependabot-miro-bundle.yml
@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging.

Re-run the job to retry the review; a new push also retries it only if the caller's pull_request triggers include synchronize (the canonical security caller keeps it). An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator.

Re-running does NOT help for every class:

  • rate-limit that persists across re-runs, or auth — the credential or usage budget needs an operator; retrying will not clear it.
  • a run that exhausted its turn budget ("subtype":"error_max_turns" above) will exhaust it again. As the PR author, split the change into smaller PRs; raising --max-turns is a change to the caller workflow, not something you can set on this PR.

@kyle-sexton
kyle-sexton merged commit fdac39e into main Aug 12, 2026
37 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/dependabot-miro-bundle-63e1 branch August 12, 2026 12:47
@github-actions

Copy link
Copy Markdown

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-run the job to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dependabot cannot land a bundled miro dependency bump: the committed dist artifact needs a manual regenerate

2 participants