Skip to content

ci: add pr template check workflow#21931

Merged
sapphi-red merged 1 commit intovitejs:mainfrom
sapphi-red:ci/add-pr-template-check-workflow
Mar 23, 2026
Merged

ci: add pr template check workflow#21931
sapphi-red merged 1 commit intovitejs:mainfrom
sapphi-red:ci/add-pr-template-check-workflow

Conversation

@sapphi-red
Copy link
Member

Adds a workflow that checks that the PR description follow the requirements in the template. It doesn't add a comment for now s o that it won't be noisy until we evaluate it.

refs #21630

@sapphi-red sapphi-red added the p1-chore Doesn't change code behavior (priority) label Mar 18, 2026
Comment on lines +3 to +9
# SAFETY: pull_request_target is used here because:
# - The workflow does NOT check out PR code (actions/checkout checks out the base branch)
# - Only PR metadata (title, body) from the event payload is read
# - No PR-supplied code is executed
on:
pull_request_target:
types: [opened]
Copy link
Member Author

Choose a reason for hiding this comment

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

This workflow uses pull_request_target which known to be often used as an attack vector. This is required so that it has access to secrets.WARP_API_KEY. I thought deeply about this safety conditions and I think it is safe. But I'm also fine to close this PR as not worth the risk.

Copy link
Member

Choose a reason for hiding this comment

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

Since we read the title and body and put it in the prompt, could that be used as an attack vector to somehow extract stuff?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is possible to steal the WARP_API_KEY but other than that, there shouldn't be any other information.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I guess this isn't any different than the clarity workflow we had before, but also, I don't think there's another way to use oz without exposing the key.

Comment on lines +90 to +95
if (agentText) {
await core.summary
.addHeading('Pull Request Template Check')
.addCodeBlock(agentText, 'json')
.write();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Silent failure when agent output format is unexpected. If no line with type: 'agent' is found in the output, agentText remains null and the workflow completes successfully without writing anything to the summary or indicating any issue.

This masks problems with the agent output format and makes debugging difficult. Should add:

if (!agentText) {
  core.setFailed('No agent output found in response');
  return;
}

Before line 91 to explicitly fail when the expected output format is not found.

Suggested change
if (agentText) {
await core.summary
.addHeading('Pull Request Template Check')
.addCodeBlock(agentText, 'json')
.write();
}
if (!agentText) {
core.setFailed('No agent output found in response');
return;
}
await core.summary
.addHeading('Pull Request Template Check')
.addCodeBlock(agentText, 'json')
.write();

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will follow up in a separate PR

@sapphi-red sapphi-red merged commit 4f7e521 into vitejs:main Mar 23, 2026
13 checks passed
@sapphi-red sapphi-red deleted the ci/add-pr-template-check-workflow branch March 23, 2026 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p1-chore Doesn't change code behavior (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants