Unblock the workspace clone; record the token exposure honestly - #183
Conversation
Reverts the escaping half of e8c8ef5 and keeps the heap fix. The escape worked for the log and broke the build. Deploy 7d4fdfae printed the instruction with $THEOREM_GIT_TOKEN as a name rather than a value, which was the point, and then failed at exit 128 because the backslash survives into the command and git receives the literal string instead of the token. Half a fix that stops every deploy is worse than the exposure it closes, and main could not deploy at all while it stood. Railway's builder takes no --secret, so RUN --mount=type=secret is not available, and their guidance is exactly what this stage already does: ARG plus one RUN in a throwaway stage whose layers never ship. So the exposure is bounded rather than removed, and the comment now says that plainly instead of claiming the opposite. The PAT is published in the logs of every build to date. Rotate it, then close this properly against the new one, where a failed experiment costs a build cycle rather than a credential.
There was a problem hiding this comment.
Pull request overview
This PR updates the workspace Dockerfile’s editor-substrate build stage to unblock cloning the private Theorem repository during image builds, while explicitly documenting that the build-time token exposure remains and must be handled operationally (rotation).
Changes:
- Rewords the Dockerfile comments to acknowledge the build-log exposure of
THEOREM_GIT_TOKENrather than claiming it is prevented. - Switches the token presence check to use the runtime-expanded value (
"${THEOREM_GIT_TOKEN}") instead of an escaped literal. - Replaces the prior credential-helper approach with a token-in-URL
git cloneto avoid the exit-128 failure introduced by escaping.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| && git clone --depth 1 --branch "${THEOREM_GIT_REF}" \ | ||
| "https://x-access-token:${THEOREM_GIT_TOKEN}@github.com/Travis-Gilbert/Theorem.git" /src \ |
| # KNOWN EXPOSURE, deliberately left in place for now. BuildKit prints each RUN | ||
| # instruction with its build args already substituted, so the clone URL below | ||
| # copies THEOREM_GIT_TOKEN verbatim into the build log, which outlives the build | ||
| # and is readable by anyone with access to the project. Treat this PAT as |
![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: 7f1dde1d48
ℹ️ 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".
| -c credential.helper='!f() { echo username=x-access-token; echo password=\$THEOREM_GIT_TOKEN; }; f' \ | ||
| clone --depth 1 --branch "${THEOREM_GIT_REF}" "${THEOREM_GIT_URL}" /src \ | ||
| && git clone --depth 1 --branch "${THEOREM_GIT_REF}" \ | ||
| "https://x-access-token:${THEOREM_GIT_TOKEN}@github.com/Travis-Gilbert/Theorem.git" /src \ |
There was a problem hiding this comment.
When a deployment passes THEOREM_GIT_URL to select a mirror or alternate fork, this hardcoded URL ignores it and clones Travis-Gilbert/Theorem instead, potentially building the wrong source or failing when THEOREM_GIT_REF exists only in the configured repository. The stage still declares THEOREM_GIT_URL as a supported build argument, and the previous clone command used it, so the authenticated clone should preserve that configured source.
Useful? React with 👍 / 👎.
|
Warning Review limit reached
Next review available in: 9 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reverts the escaping half of #182 and keeps the heap fix. Main cannot deploy the workspace service while that escaping stands.
The escape worked for the log and broke the build. Deploy
7d4fdfaeprinted the instruction with$THEOREM_GIT_TOKENas a name rather than a value, which was the whole point, and then failed at exit 128: the backslash survives into the command, so git receives the literal string instead of the token. Half a fix that stops every deploy is worse than the exposure it closes.Railway's builder takes no
--secret, soRUN --mount=type=secretis unavailable, and their own guidance is exactly what this stage already does:ARGplus oneRUNin a throwaway stage whose layers never ship.So the exposure is bounded rather than removed, and the comment now says so plainly instead of claiming the opposite (it previously read "Never echo it" directly above the line that echoed it).
Action required
THEOREM_GIT_TOKENis in the logs of every build to date. Rotate it. Then closing this properly costs a build cycle rather than a credential.Also still exposed and worth rotating from earlier in this session:
WORKSPACE_TOKEN,THEOREM_ACP_TOKEN.Still failing after this
This unblocks the clone. The Studio stage's own OOM fix (heap ceiling 12288MiB) is retained from #182 and remains unverified - no build has yet got past the clone to exercise it.