Wire runtime credentials into workforce deploy#171
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for resolving and injecting runtime credentials during deployment (specifically in dev mode or with a BYO sandbox). It adds a helper resolveRuntimeCredentialEnv to fetch credentials from the cloud, injects them into the launcher environment, and masks integration tokens to prevent leakage. It also includes unit and live integration tests to verify this behavior. Feedback on the changes highlights a potential TypeScript compilation error and runtime crash under strictNullChecks because activeToken can be undefined but is passed to a parameter typed strictly as string. A suggestion is provided to make workspaceToken optional and handle its absence gracefully.
| async function resolveRuntimeCredentialEnv(args: { | ||
| mode: DeployMode; | ||
| persona: PersonaSpec; | ||
| workspace: string; | ||
| workspaceToken: string; | ||
| cloudUrl?: string; | ||
| byoSandbox: boolean; | ||
| enabled: boolean; | ||
| }): Promise<Record<string, string> | undefined> { | ||
| if (!args.enabled || !shouldRequestRuntimeCredentials(args)) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
The activeToken variable can be undefined (as indicated by its conditional usage on line 319). Passing it to resolveRuntimeCredentialEnv where workspaceToken is strictly typed as string will cause a TypeScript compilation error under strictNullChecks.
Furthermore, if activeToken is missing (e.g., in offline or unauthenticated local dev scenarios), attempting to fetch runtime credentials from the cloud will fail with a 401 Unauthorized or network error, crashing the entire deployment. Making workspaceToken optional and returning undefined early when it is missing prevents this crash and allows local-only dev flows to proceed smoothly.
| async function resolveRuntimeCredentialEnv(args: { | |
| mode: DeployMode; | |
| persona: PersonaSpec; | |
| workspace: string; | |
| workspaceToken: string; | |
| cloudUrl?: string; | |
| byoSandbox: boolean; | |
| enabled: boolean; | |
| }): Promise<Record<string, string> | undefined> { | |
| if (!args.enabled || !shouldRequestRuntimeCredentials(args)) { | |
| return undefined; | |
| } | |
| async function resolveRuntimeCredentialEnv(args: { | |
| mode: DeployMode; | |
| persona: PersonaSpec; | |
| workspace: string; | |
| workspaceToken?: string; | |
| cloudUrl?: string; | |
| byoSandbox: boolean; | |
| enabled: boolean; | |
| }): Promise<Record<string, string> | undefined> { | |
| if (!args.enabled || !args.workspaceToken || !shouldRequestRuntimeCredentials(args)) { | |
| return undefined; | |
| } |
khaliqgant
force-pushed
the
codex/conn-runtime-credentials-acceptance
branch
3 times, most recently
from
7375a3d to
c2a529a
Compare
khaliqgant
force-pushed
the
codex/conn-runtime-credentials-acceptance
branch
from
c2a529a to
8cca9ac
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/deploy/src/deploy.ts">
<violation number="1" location="packages/deploy/src/deploy.ts:510">
P1: Do not include raw runtime-credentials payloads in thrown errors; it can leak `relayfileToken` secrets into logs.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| throw new Error( | ||
| `runtime-credentials response missing relayfileUrl/relayfileWorkspaceId/relayfileToken: ${JSON.stringify(body)}` | ||
| ); |
There was a problem hiding this comment.
P1: Do not include raw runtime-credentials payloads in thrown errors; it can leak relayfileToken secrets into logs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/deploy/src/deploy.ts, line 510:
<comment>Do not include raw runtime-credentials payloads in thrown errors; it can leak `relayfileToken` secrets into logs.</comment>
<file context>
@@ -370,13 +381,159 @@ function defaultIntegrationResolver(args: {
+ ? body.relayfileMountPaths.filter((entry): entry is string => typeof entry === 'string')
+ : [];
+ if (!relayfileUrl || !relayfileWorkspaceId || relayfileToken === '') {
+ throw new Error(
+ `runtime-credentials response missing relayfileUrl/relayfileWorkspaceId/relayfileToken: ${JSON.stringify(body)}`
+ );
</file context>
| throw new Error( | |
| `runtime-credentials response missing relayfileUrl/relayfileWorkspaceId/relayfileToken: ${JSON.stringify(body)}` | |
| ); | |
| const safeBody = { | |
| relayfileUrl: body.relayfileUrl, | |
| relayfileWorkspaceId: body.relayfileWorkspaceId, | |
| relayfileToken: body.relayfileToken === null ? null : '[redacted]', | |
| relayfileMountPaths: body.relayfileMountPaths | |
| }; | |
| throw new Error( | |
| `runtime-credentials response missing relayfileUrl/relayfileWorkspaceId/relayfileToken: ${JSON.stringify(safeBody)}` | |
| ); |
|
Reviewed PR #171 and made two fixes:
Local verification completed:
|
|
✅ pr-reviewer applied fixes — committed and pushed Reviewed PR #171 and made two fixes:
Local verification completed:
|
![agent-relay-bot[bot]](https://avatars.githubusercontent.com/in/3624277?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ pr-reviewer applied fixes — committed and pushed 05262c2 to this PR. The notes below describe what changed.
Reviewed PR #171 and made two fixes:
- Gated
packages/deploy/src/runtime-credentials-live.test.tsbehindWORKFORCE_LIVE_RUNTIME_CREDENTIALS_TESTS=1so normal test runs don’t depend on production network behavior. - Fixed
packages/runtime/src/ctx.test.tsisolation by clearingRELAYFILE_TOKENandRELAY_AGENT_TOKENSin the “cloud auth absent” case.
Local verification completed:
corepack pnpm --filter @agentworkforce/deploy testcorepack pnpm run check
|
Reviewed PR #171 and made one CI-stability fix: Verification run locally:
Deploy tests pass with the live runtime-credentials probe skipped by default. |
There was a problem hiding this comment.
Reviewed PR #171 and made one CI-stability fix: packages/deploy/src/runtime-credentials-live.test.ts now runs only when WORKFORCE_LIVE_TESTS=1, so normal test runs don’t depend on the live agentrelay.com endpoint.
Verification run locally:
corepack pnpm --filter @agentworkforce/persona-kit buildcorepack pnpm --filter @agentworkforce/deploy testcorepack pnpm --filter @agentworkforce/deploy lint
Deploy tests pass with the live runtime-credentials probe skipped by default.
|
Fixed the issues I found:
Validation run locally:
|
There was a problem hiding this comment.
Fixed the issues I found:
- Gated the live runtime-credentials probe in runtime-credentials-live.test.ts behind
WORKFORCE_LIVE_RUNTIME_CREDENTIALS_TEST=1, so normal tests do not depend on an external service. - Hardened the runtime no-auth memory test in ctx.test.ts to clear all token/workspace fallback env vars used by the runtime.
Validation run locally:
corepack pnpm --filter @agentworkforce/deploy testcorepack pnpm --filter @agentworkforce/runtime testcorepack pnpm run check
|
Reviewed PR #171 locally and made targeted fixes:
Local verification passed:
|
|
Reviewed PR #171 locally and made targeted fixes:
Local verification passed:
|
There was a problem hiding this comment.
Reviewed PR #171 locally and made targeted fixes:
- Prevented malformed runtime-credentials errors from dumping response bodies that may contain scoped tokens.
- Added a regression test for that non-leak behavior.
- Gated the new live runtime-credentials probe behind
WORKFORCE_LIVE_RUNTIME_CREDENTIALS_TEST=1so default CI is deterministic. - Fixed an existing runtime test isolation gap by clearing all token fallback env vars.
Local verification passed:
corepack pnpm --filter @agentworkforce/deploy testcorepack pnpm --filter @agentworkforce/runtime testcorepack pnpm run check
|
Implemented the PR review fixes:
Local verification passed:
|
There was a problem hiding this comment.
Implemented the PR review fixes:
- Gated the live runtime-credentials probe behind
WORKFORCE_LIVE_RUNTIME_CREDENTIALS_TEST=1. - Removed malformed runtime-credentials response body dumping so scoped tokens cannot leak through error messages.
- Added a regression test for malformed response token non-leakage.
- Hardened the runtime no-auth memory test by clearing additional token fallback env vars.
Local verification passed:
corepack pnpm --filter @agentworkforce/deploy testcorepack pnpm --filter @agentworkforce/runtime testcorepack pnpm run check
|
Reviewed PR #171 and made the needed fixes. Changed:
Validation:
|
There was a problem hiding this comment.
Reviewed PR #171 and made the needed fixes.
Changed:
- Made
packages/deploy/src/runtime-credentials-live.test.tsopt-in withWORKFORCE_LIVE_RUNTIME_CREDENTIALS=1, so normal tests do not depend on the public cloud route. - Fixed
packages/runtime/src/ctx.tssoRELAYFILE_TOKENpath-scoped runtime credentials are not reused as cloud memory auth. - Added runtime coverage proving relayfile path tokens do not enable memory calls, and hardened the no-auth memory test against ambient env.
Validation:
corepack pnpm --filter @agentworkforce/deploy testpassed.corepack pnpm --filter @agentworkforce/runtime testpassed.corepack pnpm run checkpassed locally.
|
Reviewed PR #171 and applied the needed fixes. Changed:
Local verification passed:
|
|
Reviewed PR #171 and applied the needed fixes. Changed:
Local verification passed:
|
There was a problem hiding this comment.
Reviewed PR #171 and applied the needed fixes.
Changed:
- Gated the live runtime-credentials probe behind
WORKFORCE_LIVE_RUNTIME_CREDENTIALS=1. - Removed malformed runtime-credentials response body dumping so scoped tokens cannot leak in errors.
- Added deploy regression coverage for malformed response non-leakage.
- Fixed runtime memory auth so
RELAYFILE_TOKENpath-scoped credentials are not reused as cloud memory auth. - Hardened the no-auth memory test against ambient fallback env vars and added regression coverage.
Local verification passed:
corepack pnpm --filter @agentworkforce/deploy testcorepack pnpm --filter @agentworkforce/runtime testcorepack pnpm run check
1 participant
Summary
/runtime-credentialsfor non-cloud deploy launches with declared integrationsrelay_pametadata into launched dev/BYO-sandbox personasEvidence
corepack pnpm --filter @agentworkforce/deploy test-> 153/153 passgit diff --check-> passContract coverage
packages/deploy/src/runtime-credentials-live.test.tsasserts prod POST no-auth returns 401unauthorizedand GET returns 405.WORKFORCE_INTEGRATION_GITHUB_TOKEN=WORKFORCE_PROVIDER_TOKEN_SHOULD_NOT_LEAKand asserts the launched env, request calls, and logs do not contain the sentinel; provider token/connection env values are masked.githubwriteback trigger to/runtime-credentials, asserts request includes the same trigger, and asserts launched env receives non-nullrelay_pa_scopedplus matching/github/repos/**/**/pulls/**mount paths.relayfileToken:nullas emptyRELAYFILE_TOKENandRELAYFILE_MOUNT_PATHS=[].No hosted cloud deployment endpoint behavior changed; this is the workforce deploy half only.