Add a code review step which uses a short-lived provider token#7932
Add a code review step which uses a short-lived provider token#7932
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6cb8ae9c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab11ec115a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if [ ! -f "$COMMENTS" ]; then | ||
| echo "No comments produced, skipping review submission." | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
This early return skips submission whenever comments.json is missing, even if conclusion.json exists with a non-empty body from finish_review. The recipe prompt explicitly allows summary-only output via finish_review, so in that path the workflow silently drops the generated review instead of posting it.
Useful? React with 👍 / 👎.
| function jsonResponse(status, body, extraHeaders = {}) { | ||
| return new Response(JSON.stringify(body), { | ||
| status, | ||
| headers: { "Content-Type": "application/json", ...extraHeaders }, |
There was a problem hiding this comment.
Error paths (for example invalid token or rate limiting) use jsonResponse, but this helper only sets Content-Type and omits Access-Control-Allow-Origin. In cross-origin browser usage, those 401/429 responses are blocked by CORS even though preflight and proxied success responses allow CORS, which prevents clients from reading the error payload or handling retry/auth flows correctly.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,84 @@ | |||
| #!/usr/bin/env -S uv run --script | |||
There was a problem hiding this comment.
nice and brief! was wondering if a skill and gh cli could do this but this is more crisp
There was a problem hiding this comment.
That is possible, but if goose is to invoke the gh cli directly, we need to give it a GITHUB_TOKEN, which would then be vulnerable to stealing via promp injection. So instead we give goose the least privilege/secrets possible (only the short-lived token for the LLM proxy) and have it stage the whole review to an artifact, which is later uploaded by a step with the right privilege.
We could just have goose write to a json file directly, but that felt like asking more of the LLM vs giving it tools.
DOsinga
added
the
needs_human
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
I like it. Does this mean we can also retire the codex review bot which is starting to get really noisy and harmful?
Eventually that would be great, but right now I think the codex review is probably a lot better at general code reviews. My thinking was that we use this new method to add specific things to look for, and if we can get to a point where the prompts work well enough to do general reviews, that's a great too. |
* origin/main: fix: use pnpm when publishing (#8092) Alexhancock/publish npm file format fix (#8091) fix: iteration on the publish-npm workflow (#8087) chore: ignore unmaintained warning for proc-macro-error (#8084) chore: clean up stray recipe.yaml (#8086) chore(release): bump version to 1.29.0 (minor) (#8088) Update lockfile references (#8085) Fix version bump (#8083) Add a code review step which uses a short-lived provider token (#7932)
* main: Fix user message text silently dropped when message contains both text and image (#8071) fix: use pnpm when publishing (#8092) Alexhancock/publish npm file format fix (#8091) fix: iteration on the publish-npm workflow (#8087) chore: ignore unmaintained warning for proc-macro-error (#8084) chore: clean up stray recipe.yaml (#8086) chore(release): bump version to 1.29.0 (minor) (#8088) Update lockfile references (#8085) Fix version bump (#8083) Add a code review step which uses a short-lived provider token (#7932)
* origin/main: Fix user message text silently dropped when message contains both text and image (#8071) fix: use pnpm when publishing (#8092) Alexhancock/publish npm file format fix (#8091) fix: iteration on the publish-npm workflow (#8087) chore: ignore unmaintained warning for proc-macro-error (#8084) chore: clean up stray recipe.yaml (#8086) chore(release): bump version to 1.29.0 (minor) (#8088) Update lockfile references (#8085) Fix version bump (#8083) Add a code review step which uses a short-lived provider token (#7932)
4 participants
Summary
Adds a new code review step which proxies its LLM requests using a short-lived Github OIDC token. This lets us make provider calls from an untrusted context for e.g. code reviews (included in this PR) or live provider integration tests.
This PR also contains the source code for a cloudflare worker that can be used to do this proxying.