fix: log streaming for Capgo Builder (public)#1510
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds optional HS256 JWT generation (4h) for direct SSE build log streaming, emits a deprecation notice for the logs proxy stream, updates an encrypted-file mapping, and adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BuildStart as Build Start Function
participant JWTGen as JWT Generator
participant CloudLog
participant DirectSSE as Direct Log Stream (SSE)
Client->>BuildStart: POST /build/start
BuildStart->>BuildStart: create job (jobId)
BuildStart->>JWTGen: generateLogStreamToken(jobId, userId, appId, JWT_SECRET)
alt JWT secret present & token created
JWTGen-->>BuildStart: logs_token
BuildStart->>CloudLog: emit "Build logs stream request (deprecated proxy - use direct SSE)"
BuildStart-->>Client: 200 { ..., logs_url, logs_token }
else JWT secret missing or token error
JWTGen-->>BuildStart: error / skipped
BuildStart->>CloudLog: emit skip/error log
BuildStart-->>Client: 200 { ... } (no logs fields)
end
Note over Client,DirectSSE: Client may use logs_url + logs_token to connect directly to SSE log stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@supabase/functions/_backend/public/build/start.ts`:
- Around line 17-79: The generateLogStreamToken function currently accepts any
jwtSecret length—add a minimum-strength validation at the start of
generateLogStreamToken (e.g., require a minimum number of characters/entropy for
jwtSecret) and throw/return an error if it fails, referencing jwtSecret in the
check; alternatively replace the manual JWT creation with a standard library
(e.g., jose) to sign the token securely (update import/use to create an HS256
token with the provided jwtSecret and keep the same payload fields job_id,
user_id, app_id, iat, exp).
🧹 Nitpick comments (1)
supabase/functions/_backend/public/build/start.ts (1)
215-250: Good graceful degradation pattern.The try/catch around token generation with fallback to the proxy approach maintains backward compatibility. Structured logging follows the coding guidelines.
One minor concern: the hardcoded fallback
'https://api.capgo.app'forPUBLIC_URLcould cause issues in development or staging environments where the direct logs endpoint would point to production instead of the current environment.💡 Consider logging a warning when using the fallback URL
- const publicUrl = getEnv(c, 'PUBLIC_URL') || 'https://api.capgo.app' + const publicUrl = getEnv(c, 'PUBLIC_URL') + if (!publicUrl) { + cloudlog({ + requestId: c.get('requestId'), + message: 'PUBLIC_URL not configured, using default', + job_id: jobId, + }) + } + const effectivePublicUrl = publicUrl || 'https://api.capgo.app'
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@supabase/functions/_backend/public/build/start.ts`:
- Line 4: Reorder the imports so external modules come before parent/relative
imports: move the "import { SignJWT } from 'npm:jose'" line above the import of
"../../utils/hono.ts" (and any other relative imports) in start.ts; ensure
external imports (e.g., SignJWT from 'npm:jose') are grouped at the top, then
parent/relative imports (e.g., utils/hono.ts), and run the linter/formatter to
confirm the import order is correct.
🧹 Nitpick comments (1)
supabase/functions/_backend/public/build/start.ts (1)
18-35: Consider adding minimum secret length validation.The function accepts any non-empty
jwtSecretvalue. For HS256, secrets shorter than 32 characters provide weak security. The caller only checksif (jwtSecret)at line 178, so short secrets like"abc"would still produce weak tokens.🔒 Suggested validation
async function generateLogStreamToken( jobId: string, userId: string, appId: string, jwtSecret: string, ): Promise<string> { + if (jwtSecret.length < 32) { + throw new Error('JWT_SECRET must be at least 32 characters for secure signing') + } + const secret = new TextEncoder().encode(jwtSecret)
Add issuer, audience, and subject claims to the JWT token for better security. Move user_id to the standard 'sub' claim. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds support for direct log streaming from the Capgo Builder worker by introducing optional logs_url and logs_token fields to the /build/start endpoint response. This enables the CLI to bypass the existing proxy-based log streaming endpoint and connect directly to the builder worker, reducing operational costs and improving reliability.
Changes:
- Adds JWT token generation for time-limited (4h) direct log stream access
- Returns optional
logs_urlandlogs_tokenin/build/startresponse when JWT_SECRET is configured - Adds deprecation notice to existing proxy-based
/build/logsendpoint - Adds
josenpm package (v6.1.3) for JWT signing
Reviewed changes
Copilot reviewed 4 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/functions/_backend/public/build/start.ts | Implements JWT token generation and adds logs_url/logs_token to response |
| supabase/functions/_backend/public/build/logs.ts | Adds deprecation notice for proxy-based endpoint |
| package.json | Adds jose library dependency |
| bun.lock | Lock file update for jose package |
| internal/cloudflare/.env.prod.secret | Encrypted production environment configuration |
| internal/cloudflare/.env.preprod.secret | Encrypted pre-production environment configuration |
| internal/cloudflare/.env.local.secret | Encrypted local environment configuration |
| internal/supabase/.env.local.secret | Encrypted local Supabase environment configuration |
| internal/how-to-deploy.md.secret | Encrypted deployment documentation |
| internal/forgr-key.jks.secret | Encrypted Android signing key |
| internal/forgr-key.jks.base64.secret | Encrypted base64-encoded Android signing key |
| internal/capgo-394818-68ad1517d330.json.secret | Encrypted GCP service account credentials |
| internal/Certificates_p12.p12.secret | Encrypted iOS certificates |
| internal/Certificates.p12.secret | Encrypted iOS certificates |
| internal/CICD.mobileprovision.secret | Encrypted iOS provisioning profile |
| internal/AuthKey_8P7Y3V99PJ.p8.secret | Encrypted iOS auth key |
| .gitsecret/paths/mapping.cfg | Updated hash for encrypted prod environment file |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@riderx Please merge this PR. Its a good PR for rewriting the logs system :) |
|
Summary
This PR introduces a new field called
logs_urlandlogs_tokento the /build/start endpoint. This is then used by the CLI to connect to the capgo builder worker directly skipping the proxyMotivation
Martin asked me to improve and fix the native build, so I am doing just that
Business impact
Low - removing the proxy cuts costs of operating the native build + it makes the log streaming more reliable
Summary by CodeRabbit
New Features
Bug Fixes
Chores