fix(preview-deployments): Filter preview labels before the collaborator check - #4903
Open
g3n35i5 wants to merge 1 commit into
Open
fix(preview-deployments): Filter preview labels before the collaborator check#4903g3n35i5 wants to merge 1 commit into
g3n35i5 wants to merge 1 commit into
Conversation
…or check
For GitHub preview deployments the collaborator-permission check ran
*before* the preview-label filter. As a result a PR that the label filter
would exclude anyway (e.g. a `renovate[bot]` / `dependabot[bot]` PR without
the required label) still hit the permission check, was pushed to
`blockedApps`, and triggered the alarming `🚨 Preview Deployment Blocked`
security comment — even though it was never going to deploy.
The preview label is itself a valid gate: only collaborators with write
access can add labels to a PR. Evaluating it first means a PR that does not
match the configured `previewLabels` short-circuits with no GitHub API call,
no deployment, and no security comment.
Changes:
- Extract the decision logic into `apps/dokploy/utils/preview-deployment.ts`
(`pullRequestMatchesPreviewLabels` + `partitionPreviewApps`). The label
filter now runs first, so `resolveAuthorPermission` (the GitHub API call) is
only invoked for apps whose labels match the PR. The check still fails
closed: any error blocks the app rather than deploying it.
- Rework `apps/dokploy/pages/api/deploy/github.ts` to use the helper and drop
the now-redundant label check from the deploy loop.
- Improve traceability: every candidate app emits exactly one greppable
decision log — `ACCEPTED`, `BLOCKED`, or `SKIPPED` — carrying the PR number,
action, author, and reason. The previously-silent label-mismatch `continue`
is now logged as a routine skip (deliberately not a `SECURITY` block), and
the preview-limit skip is logged too. The `SECURITY` keyword is retained on
security-relevant lines so existing log-based alerting keeps matching.
Example decision logs (one per candidate app, per PR event):
⏭️ Preview SKIPPED for "web" — PR Dokploy#42 (opened) by dependency-bot[bot] on acme/web-app: PR labels [dependencies, renovate] do not match required [preview]
🚨 SECURITY: Preview BLOCKED for "web" — PR Dokploy#42 (opened) by dependency-bot[bot] on acme/web-app: author lacks write access (permission: none)
⚠️ SECURITY: Preview ACCEPTED for "web" — PR Dokploy#42 (opened) by dependency-bot[bot] on acme/web-app: collaborator permission check disabled (any author allowed)
✅ SECURITY: Preview ACCEPTED for "web" — PR Dokploy#42 (opened) by alice on acme/web-app: author has write access (permission: write)
⏭️ Preview SKIPPED for "web" — PR Dokploy#42 (opened): preview limit reached (3/3)
Tests:
- `apps/dokploy/__test__/git-provider/preview-deployment-label-order.test.ts`
(new) unit-tests the label semantics, the label-before-permission ordering,
fail-closed behavior, and the traceability log contract.
- `apps/dokploy/__test__/deploy/github-webhook-handler.test.ts` gains three
end-to-end `pull_request` cases driving the real handler: a label-mismatched
bot PR posts no security comment and does not deploy; a label-matched PR
without write access posts the comment; a label-matched PR with write access
queues the preview.
Closes Dokploy#4902
---
AI assistance: this change was drafted with Claude Code using the
`claude-opus-4-8[1m]` model (Opus 4.8, 1M-token context). The GitHub issue was
read via the `gh` CLI, the fix and tests were generated iteratively, and every
change was manually reviewed, `tsc`/Biome-checked, and verified against the
passing test suite before committing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
the
size:L
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.
What is this PR about?
For GitHub preview deployments the collaborator-permission check ran before the preview-label filter. As a result a PR that the label filter would exclude anyway (e.g. a
renovate[bot]/dependabot[bot]PR without the required label) still hit the permission check, was pushed toblockedApps, and triggered the alarming🚨 Preview Deployment Blockedsecurity comment — even though it was never going to deploy.The preview label is itself a valid gate: only collaborators with write access can add labels to a PR. Evaluating it first means a PR that does not match the configured
previewLabelsshort-circuits with no GitHub API call, no deployment, and no security comment.Changes:
apps/dokploy/utils/preview-deployment.ts(pullRequestMatchesPreviewLabels+partitionPreviewApps). The label filter now runs first, soresolveAuthorPermission(the GitHub API call) is only invoked for apps whose labels match the PR. The check still fails closed: any error blocks the app rather than deploying it.apps/dokploy/pages/api/deploy/github.tsto use the helper and drop the now-redundant label check from the deploy loop.ACCEPTED,BLOCKED, orSKIPPED— carrying the PR number, action, author, and reason. The previously-silent label-mismatchcontinueis now logged as a routine skip (deliberately not aSECURITYblock), and the preview-limit skip is logged too. TheSECURITYkeyword is retained on security-relevant lines so existing log-based alerting keeps matching.Example decision logs (one per candidate app, per PR event):
Tests:
apps/dokploy/__test__/git-provider/preview-deployment-label-order.test.ts(new) unit-tests the label semantics, the label-before-permission ordering, fail-closed behavior, and the traceability log contract.apps/dokploy/__test__/deploy/github-webhook-handler.test.tsgains three end-to-endpull_requestcases driving the real handler: a label-mismatched bot PR posts no security comment and does not deploy; a label-matched PR without write access posts the comment; a label-matched PR with write access queues the preview.Built this branch, deployed the image to a real Dokploy instance, and replayed the failing scenario — voraus-renovate[bot] PRs on vorausrobotik/vdoc without the required preview label — across opened / labeled / unlabeled events on two PRs.
Every event now short-circuits at the label filter: no permission check, no deployment, and no 🚨 Preview Deployment Blocked comment. The decision logs confirm it:
No security comment was posted on either PR — the false-positive alarm is gone.
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
Closes #4902
Screenshots (if applicable)
N/A
Note
AI assistance: this change was drafted with Claude Code using the
claude-opus-4-8[1m]model (Opus 4.8, 1M-token context).