feat: implement simple api key auth with TODOs to replace after demo#4010
feat: implement simple api key auth with TODOs to replace after demo#4010
Conversation
mcstepp
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds static API-key authentication for the ctrl service and clients, wires a Connect interceptor into ctrl handlers, propagates API keys via CLI and dashboard env/flags, extends quickstart docs for local deployment (dnsmasq, certs, PORT requirement), and updates ignore rules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant Dashboard as Dashboard
participant CTRL as ctrl Service
participant Auth as Auth Middleware
rect #e8f0ff
note right of CLI: Clients attach Authorization header when configured\n(UNKEY_API_KEY / CTRL_API_KEY)
end
CLI->>CTRL: Connect RPC (Authorization: Bearer key_xxx)
activate CTRL
CTRL->>Auth: Intercept request
alt Health / ACME endpoints
Auth-->>CTRL: Skip auth
CTRL-->>CLI: 200 OK
else Protected endpoints
Auth->>Auth: Validate Bearer token
alt Valid
Auth-->>CTRL: Forward to handler
CTRL-->>CLI: Success response
else Invalid/Missing
Auth-->>CLI: Unauthenticated error
end
end
deactivate CTRL
Dashboard->>CTRL: Connect RPC (Authorization: Bearer key_xxx)
CTRL->>Auth: Intercept and validate (same flow)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
bruh |
|
yikes it is in the gitignore but idk why it didn't do it. I cleared it from my cache though |
|
and it didn't pick up again |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
go/apps/ctrl/config.go (1)
89-98: Fail fast if CTRL_API_KEY is missing.Without a configured key, middleware will reject all but health/ACME calls. Validate here to surface misconfig at startup.
func (c Config) Validate() error { // Validate Cloudflare configuration if enabled if c.Acme.Enabled && c.Acme.Cloudflare.Enabled { if err := assert.NotEmpty(c.Acme.Cloudflare.ApiToken, "cloudflare API token is required when cloudflare is enabled"); err != nil { return err } } + // Require API key for ctrl authentication (demo mode) + if err := assert.NotEmpty(c.APIKey, "CTRL_API_KEY is required for ctrl authentication"); err != nil { + return err + } + return nil }go/apps/ctrl/run.go (1)
180-187: Hardcoded metald auth headers; parameterize and lower log level.Shipping fixed "dev_user_ctrl"/"ctrl-tenant" to main is risky and noisy. Make them env-configurable (with safe defaults for local only) and avoid info-level per-request logs.
Apply this diff:
@@ - metaldClient := metaldv1connect.NewVmServiceClient( + // Allow overriding dev auth headers via env for non-local envs + token := os.Getenv("METALD_BEARER_TOKEN") + if token == "" { + token = "dev_user_ctrl" + } + tenant := os.Getenv("METALD_TENANT_ID") + if tenant == "" { + tenant = "ctrl-tenant" + } + + metaldClient := metaldv1connect.NewVmServiceClient( @@ - connect.WithInterceptors(connect.UnaryInterceptorFunc(func(next connect.UnaryFunc) connect.UnaryFunc { + connect.WithInterceptors(connect.UnaryInterceptorFunc(func(next connect.UnaryFunc) connect.UnaryFunc { return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { - logger.Info("Adding auth headers to metald request", "procedure", req.Spec().Procedure) - req.Header().Set("Authorization", "Bearer dev_user_ctrl") - req.Header().Set("X-Tenant-ID", "ctrl-tenant") + logger.Debug("Adding metald auth headers", "procedure", req.Spec().Procedure) + req.Header().Set("Authorization", "Bearer "+token) + req.Header().Set("X-Tenant-ID", tenant) return next(ctx, req) } })), )Also add the import:
import ( "context" "database/sql" "fmt" "log/slog" "net/http" + "os" "time"QUICKSTART-DEPLOY.md (1)
5-9: Update QUICKSTART-DEPLOY.md to require Go 1.25+Change "Go 1.24 or later" → "Go 1.25 or later" (QUICKSTART-DEPLOY.md, lines 5–9); repo go.mod files (including go/go.mod) declare go 1.25.
🧹 Nitpick comments (13)
go/apps/ctrl/.gitignore (1)
1-3: Ignore secrets: good. Add exception for examples.Keep
.env*out of VCS but allow committed templates.# Environment variables with secrets -.env -.env.local +.env +.env.local +!.env.exampleapps/dashboard/lib/ctrl.ts (4)
26-35: Set Content-Type only when sending a body; add Accept.Minor header hygiene.
- const defaultHeaders: Record<string, string> = { - "Content-Type": "application/json", - // TODO: Replace API key with JWT token when moving ctrl service to private IP - Authorization: `Bearer ${ctrlApiKey}`, - }; + const defaultHeaders: Record<string, string> = { + Accept: "application/json", + // TODO: Replace API key with JWT token when moving ctrl service to private IP + Authorization: `Bearer ${ctrlApiKey}`, + }; + if (options.body !== undefined) { + defaultHeaders["Content-Type"] = "application/json"; + }
37-41: Robust URL join + request timeout.Avoid double/missing slashes and hang protection for external calls.
- const response = await fetch(`${ctrlUrl}${endpoint}`, { - method: options.method || "POST", - headers, - body: options.body ? JSON.stringify(options.body) : undefined, - }); + const url = new URL(endpoint, ctrlUrl).toString(); + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), (options as any).timeoutMs ?? 30000); + try { + const response = await fetch(url, { + method: options.method || "POST", + headers, + body: options.body !== undefined ? JSON.stringify(options.body) : undefined, + signal: controller.signal, + }); + return response; + } finally { + clearTimeout(timeout); + }
59-73: Include response body in error when not JSON.Keep current behavior but fall back to body text if JSON parse fails (already done). Optionally capture a request ID header if present (e.g.,
x-request-id) to aid debugging.
75-76: Guard 204/empty bodies.Parsing JSON on 204 will throw.
- return await response.json(); + if (response.status === 204) { + return undefined as T; + } + return (await response.json()) as T;go/apps/ctrl/middleware/auth.go (2)
40-49: Centralize unauthenticated allowlist.Use a small switch/set to avoid drift and make additions explicit.
- // Skip authentication for health check endpoint - if procedure == "/ctrl.v1.CtrlService/Liveness" { - return next(ctx, req) - } - - // Skip authentication for ACME certificate verification (public endpoint) - if procedure == "/ctrl.v1.AcmeService/HandleCertificateVerification" { - return next(ctx, req) - } + switch procedure { + case "/ctrl.v1.CtrlService/Liveness", + "/ctrl.v1.AcmeService/HandleCertificateVerification": + return next(ctx, req) + }
80-82: Reduce noise: log successes at debug level.Success on every call at info can flood logs.
- m.logAuthEvent(ctx, "success", apiKey, procedure) + // Consider downgrading to debug: + m.logAuthEvent(ctx, "success", apiKey, procedure) // change to Debug in logger or gate by log levelgo/apps/ctrl/config.go (1)
67-69: Clarify or consolidate AuthToken vs APIKeyAuthToken is used as the bearer token across deploy/agent code; APIKey is a simple/demo key used only by the ctrl service and dashboard — the overlap is confusing. Rename or clearly document/deprecate one.
- Defined together: go/apps/ctrl/config.go (AuthToken, APIKey).
- APIKey usage: go/apps/ctrl/middleware/auth.go, go/apps/ctrl/run.go, QUICKSTART-DEPLOY.md (CTRL_API_KEY), apps/dashboard/lib/ctrl.ts.
- AuthToken usage: go/cmd/deploy/control_plane.go, go/cmd/ctrl/main.go, apps/agent/pkg/cluster/cluster.go, apps/agent/pkg/config/agent.go, apps/agent/pkg/api/server.go, apps/agent/services/ratelimit/*.
Action: either rename APIKey (e.g., DemoAPIKey/CTRL_API_KEY) and add an explicit comment that it’s demo-only, or deprecate/remove it and consolidate on AuthToken; update flags/docs/quickstart accordingly.
go/apps/ctrl/run.go (1)
214-218: Clarify/adjust log when no API key is set.Message implies broad rejections; consider adding “set CTRL_API_KEY to enable” and severity Warn→Info in local/dev.
- logger.Warn("No API key configured - authentication will reject all requests except health check and ACME routes") + logger.Warn("No API key configured: auth will reject all requests except health/ACME. Set CTRL_API_KEY to enable auth")QUICKSTART-DEPLOY.md (4)
14-18: .env example: avoid quotes to prevent accidental inclusion.-CTRL_API_KEY="your-local-dev-key" +CTRL_API_KEY=your-local-dev-key
37-48: List numbering is inconsistent.Use 1. for all items or fix numbering (2 → 4 after insertion).
80-85: Mention alternative CLI env name supported (if applicable).Docs elsewhere reference UNKEY_CTRL_API_KEY; add it for clarity.
-export API_KEY=your-local-dev-key +# Either of these (CLI prefers API_KEY if both set): +export API_KEY=your-local-dev-key +# export UNKEY_CTRL_API_KEY=your-local-dev-key
86-95: CLI flags: good; add hint to export CTRL_URL to avoid repeating the flag.go run . deploy \ --context=./demo_api \ --workspace-id="REPLACE_ME" \ --project-id="REPLACE_ME" \ --control-plane-url="http://127.0.0.1:7091" \ --keyspace-id="REPLACE_ME" # This is optional if you want key verifications + +# Tip: +# export CTRL_URL=http://127.0.0.1:7091 # then omit --control-plane-url
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
deployment/data/metald/metald.dbis excluded by!**/*.dbpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
QUICKSTART-DEPLOY.md(2 hunks)apps/dashboard/lib/ctrl.ts(1 hunks)apps/dashboard/lib/env.ts(1 hunks)go/apps/ctrl/.gitignore(1 hunks)go/apps/ctrl/config.go(1 hunks)go/apps/ctrl/middleware/auth.go(1 hunks)go/apps/ctrl/run.go(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-02T11:09:58.791Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/trpc/routers/key/create.ts:11-14
Timestamp: 2025-06-02T11:09:58.791Z
Learning: In the unkey codebase, TypeScript and the env() function implementation already provide sufficient validation for environment variables, so additional runtime error handling for missing env vars is not needed.
Applied to files:
apps/dashboard/lib/env.ts
📚 Learning: 2025-09-15T18:12:01.503Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: go/apps/ctrl/run.go:212-213
Timestamp: 2025-09-15T18:12:01.503Z
Learning: In Go connect-go generated handlers, mux.Handle can accept New*ServiceHandler functions directly even though they return (string, http.Handler) tuples. The pattern mux.Handle(ctrlv1connect.New*ServiceHandler(...)) is valid and compiles successfully in unkey codebase.
Applied to files:
go/apps/ctrl/run.go
🧬 Code graph analysis (3)
apps/dashboard/lib/ctrl.ts (2)
apps/dashboard/lib/env.ts (1)
env(3-52)apps/dashboard/lib/collections/client.ts (1)
fetch(17-22)
go/apps/ctrl/run.go (8)
go/apps/ctrl/middleware/auth.go (2)
NewAuthMiddleware(28-32)AuthConfig(13-19)go/pkg/otel/logging/interface.go (1)
Logger(11-116)go/gen/proto/ctrl/v1/ctrlv1connect/service.connect.go (1)
NewCtrlServiceHandler(85-101)go/apps/ctrl/services/deployment/service.go (1)
New(18-26)go/apps/ctrl/services/openapi/service.go (1)
New(15-21)go/gen/proto/ctrl/v1/ctrlv1connect/deployment.connect.go (1)
NewDeploymentServiceHandler(126-158)go/gen/proto/ctrl/v1/ctrlv1connect/openapi.connect.go (1)
NewOpenApiServiceHandler(86-102)go/gen/proto/ctrl/v1/ctrlv1connect/acme.connect.go (1)
NewAcmeServiceHandler(86-102)
go/apps/ctrl/middleware/auth.go (1)
go/pkg/otel/logging/interface.go (1)
Logger(11-116)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Vercel Agent Review
- GitHub Check: autofix
🔇 Additional comments (4)
apps/dashboard/lib/env.ts (1)
31-33: Docs/PR references NEXT_PUBLIC_ but the codebase uses server-only CTRL_API_KEY / CTRL_URL — keep these server-side.*
Search shows server-only usage in QUICKSTART-DEPLOY.md, deployment/docker-compose.yaml, go/k8s/manifests/dashboard.yaml, tools/local/src/cmd/dashboard.ts, apps/dashboard/lib/env.ts, and apps/dashboard/lib/ctrl.ts; update PR/docs/CLI text to reference CTRL_API_KEY/CTRL_URL (do not introduce NEXT_PUBLIC_ which would expose secrets).Likely an incorrect or invalid review comment.
go/apps/ctrl/run.go (3)
207-213: Auth middleware creation: confirm empty-key semantics.With cfg.APIKey empty, interceptor still attaches and relies on internal allowlist (health/ACME). Confirm this “fail-closed by default” is intended for local dev, or gate with a dev flag.
220-224: LGTM: single place for connect options.Centralizing the interceptor via connectOptions keeps handlers consistent.
225-227: Auth interceptor bypass for Liveness/ACME verified — add tests
- Verified: AuthMiddleware.ConnectInterceptor explicitly skips "/ctrl.v1.CtrlService/Liveness" and "/ctrl.v1.AcmeService/HandleCertificateVerification" (go/apps/ctrl/middleware/auth.go).
- Verified: Ctrl handlers are mounted with authInterceptor via connectOptions in go/apps/ctrl/run.go.
- Action: Add/confirm tests:
- CtrlService.Liveness reachable without Authorization.
- AcmeService.HandleCertificateVerification reachable without Authorization.
- Other procedures return 401/403 for missing or invalid API key (test both with cfg.APIKey set and with it empty).
mcstepp
force-pushed
the
ENG-2082-ctrl-keys
branch
from
c7ac21f to
44f181a
Compare
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments:
go/cmd/ctrl/main.go (line 122):
The ctrl service Config struct includes an APIKey field but there's no CLI flag defined to populate it, causing authentication to always fail since the middleware will receive an empty API key.
View Details
📝 Patch Details
diff --git a/go/cmd/ctrl/main.go b/go/cmd/ctrl/main.go
index 0ebd58cba..7c8ad9baf 100644
--- a/go/cmd/ctrl/main.go
+++ b/go/cmd/ctrl/main.go
@@ -54,6 +54,8 @@ var Cmd = &cli.Command{
// Control Plane Specific
cli.String("auth-token", "Authentication token for control plane API access. Required for secure deployments.",
cli.EnvVar("UNKEY_AUTH_TOKEN")),
+ cli.String("api-key", "API key for simple authentication",
+ cli.EnvVar("UNKEY_CTRL_API_KEY")),
cli.String("metald-address", "Full URL of the metald service for VM operations. Required for deployments. Example: https://metald.example.com:8080",
cli.Required(), cli.EnvVar("UNKEY_METALD_ADDRESS")),
cli.String("spiffe-socket-path", "Path to SPIFFE agent socket for mTLS authentication. Default: /var/lib/spire/agent/agent.sock",
@@ -120,6 +122,7 @@ func action(ctx context.Context, cmd *cli.Command) error {
// Control Plane Specific
AuthToken: cmd.String("auth-token"),
+ APIKey: cmd.String("api-key"),
MetaldAddress: cmd.String("metald-address"),
SPIFFESocketPath: cmd.String("spiffe-socket-path"),
Analysis
Missing CLI flag for APIKey causes authentication to always fail
What fails: AuthMiddleware.ConnectInterceptor() in go/apps/ctrl/middleware/auth.go rejects all authenticated requests because cfg.APIKey is always empty - no CLI flag exists to populate it
How to reproduce:
# Try to start ctrl service and make an authenticated request
go run . run ctrl --database-primary="test" --database-partition="test" --database-hydra="test" --metald-address="test"
# Service starts but logs: "No API key configured - authentication will reject all requests except health check and ACME routes"
# Any request with Authorization header will return 401 UnauthenticatedResult: All API requests fail with "Invalid API key" error, even with valid tokens, because middleware compares against empty cfg.APIKey
Expected: Should be able to set API key via --api-key flag (similar to existing --auth-token flag) so authentication middleware can validate requests properly
Fix: Added --api-key CLI flag definition and config population in go/cmd/ctrl/main.go
mcstepp
force-pushed
the
ENG-2082-ctrl-keys
branch
from
44f181a to
bc499fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
go/cmd/deploy/main.go (1)
128-129: Support env var alias for CLI (UNKEY_CTRL_API_KEY) to match docs.Currently only API_KEY is read. Add a fallback to read UNKEY_CTRL_API_KEY when the flag/env is unset.
Apply this diff:
func DeployAction(ctx context.Context, cmd *cli.Command) error { @@ opts := DeployOptions{ @@ - APIKey: cmd.String("api-key"), + APIKey: cmd.String("api-key"), Linux: cmd.Bool("linux"), } + + // Fallback to UNKEY_CTRL_API_KEY for compatibility with docs/envs + if opts.APIKey == "" { + if v := os.Getenv("UNKEY_CTRL_API_KEY"); v != "" { + opts.APIKey = v + } + }Add import:
import ( "os" )tools/local/src/cmd/dashboard.ts (2)
36-38: Prefer env override for the local dev key.Allow CTRL_API_KEY to be supplied via environment while keeping the same default.
ControlPlane: { CTRL_URL: "http://localhost:7091", - CTRL_API_KEY: "your-local-dev-key", + CTRL_API_KEY: process.env.CTRL_API_KEY ?? "your-local-dev-key", },
54-55: Avoid printing full .env contents (may include secrets later).Drop the note to reduce accidental leakage in terminals/logs.
- clack.note(env, envPath); + // Omit printing full env content to avoid exposing secrets in logsdeployment/docker-compose.yaml (2)
347-349: Use env_file for ctrl to align with docs and avoid duplication.Compose doesn’t currently read go/apps/ctrl/.env, making QUICKSTART step misleading. Load it via env_file and remove the hardcoded key here.
environment: UNKEY_DATABASE_PRIMARY: "unkey:password@tcp(mysql:3306)/unkey?parseTime=true&interpolateParams=true" @@ UNKEY_VAULT_MASTER_KEYS: "Ch9rZWtfMmdqMFBJdVhac1NSa0ZhNE5mOWlLSnBHenFPENTt7an5MRogENt9Si6wms4pQ2XIvqNSIgNpaBenJmXgcInhu6Nfv2U=" - - # API key for simple authentication (temporary, will be replaced with JWT) - UNKEY_API_KEY: "your-local-dev-key" + + # Load ctrl env (e.g., UNKEY_API_KEY) from file for local/dev + env_file: + - ../go/apps/ctrl/.env
403-405: Avoid duplicating dashboard CTRL_ config (already in env_file).*Rely on apps/dashboard/.env; remove overrides to prevent drift.
- CTRL_URL: "http://ctrl:7091" - CTRL_API_KEY: "your-local-dev-key" + # CTRL_URL / CTRL_API_KEY come from env_fileQUICKSTART-DEPLOY.md (1)
89-99: Minor: flag/env guidance clarity.Note explicitly that the CLI reads API_KEY (and now also UNKEY_CTRL_API_KEY if you accept the code change).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.gitignore(1 hunks)QUICKSTART-DEPLOY.md(2 hunks)apps/dashboard/.env.example(1 hunks)apps/dashboard/lib/env.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts(1 hunks)deployment/docker-compose.yaml(2 hunks)go/apps/ctrl/.gitignore(1 hunks)go/apps/ctrl/config.go(1 hunks)go/apps/ctrl/middleware/auth.go(1 hunks)go/apps/ctrl/run.go(2 hunks)go/cmd/ctrl/main.go(2 hunks)go/cmd/deploy/control_plane.go(3 hunks)go/cmd/deploy/main.go(3 hunks)tools/local/src/cmd/dashboard.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- go/apps/ctrl/.gitignore
- apps/dashboard/lib/env.ts
- .gitignore
- go/apps/ctrl/config.go
- go/cmd/ctrl/main.go
- apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts
- go/cmd/deploy/control_plane.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: go/cmd/deploy/control_plane.go:64-69
Timestamp: 2025-09-25T15:12:21.699Z
Learning: The API key authentication system in go/cmd/deploy/control_plane.go and related files is temporary demo code that will be replaced with a proper authentication system for private beta.
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
📚 Learning: 2025-09-24T18:57:34.843Z
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
Applied to files:
apps/dashboard/.env.exampletools/local/src/cmd/dashboard.tsdeployment/docker-compose.yamlQUICKSTART-DEPLOY.md
📚 Learning: 2025-09-15T18:12:01.503Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: go/apps/ctrl/run.go:212-213
Timestamp: 2025-09-15T18:12:01.503Z
Learning: In Go connect-go generated handlers, mux.Handle can accept New*ServiceHandler functions directly even though they return (string, http.Handler) tuples. The pattern mux.Handle(ctrlv1connect.New*ServiceHandler(...)) is valid and compiles successfully in unkey codebase.
Applied to files:
go/apps/ctrl/run.go
📚 Learning: 2025-09-25T15:12:21.699Z
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: go/cmd/deploy/control_plane.go:64-69
Timestamp: 2025-09-25T15:12:21.699Z
Learning: The API key authentication system in go/cmd/deploy/control_plane.go and related files is temporary demo code that will be replaced with a proper authentication system for private beta.
Applied to files:
go/cmd/deploy/main.gogo/apps/ctrl/middleware/auth.go
📚 Learning: 2025-09-16T19:08:44.174Z
Learnt from: Flo4604
PR: unkeyed/unkey#3980
File: go/k8s/manifests/dashboard.yaml:41-42
Timestamp: 2025-09-16T19:08:44.174Z
Learning: For local development Kubernetes manifests (typically indicated by paths containing "local" or environment variables like UNKEY_REGION: "local"), hardcoded generic credentials in environment variables are acceptable for convenience. Security recommendations about using Secrets should be reserved for production or staging environments.
Applied to files:
deployment/docker-compose.yaml
📚 Learning: 2025-09-11T14:12:30.570Z
Learnt from: Flo4604
PR: unkeyed/unkey#3944
File: go/apps/ctrl/run.go:292-299
Timestamp: 2025-09-11T14:12:30.570Z
Learning: In go/apps/ctrl/config.go, the Validate() method already includes validation for Cloudflare API token using assert.NotEmpty() when both c.Acme.Enabled and c.Acme.Cloudflare.Enabled are true, with the error message "cloudflare API token is required when cloudflare is enabled".
Applied to files:
go/apps/ctrl/middleware/auth.go
📚 Learning: 2025-09-12T18:11:33.481Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/lib/trpc/routers/deploy/rollback.ts:23-24
Timestamp: 2025-09-12T18:11:33.481Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/rollback.ts, the CTRL_URL environment variable should fail fast with a clear error message if missing in non-development environments, rather than defaulting to localhost which can mask production configuration issues.
Applied to files:
QUICKSTART-DEPLOY.md
🧬 Code graph analysis (2)
go/apps/ctrl/run.go (5)
go/apps/ctrl/middleware/auth.go (2)
NewAuthMiddleware(24-28)AuthConfig(12-15)go/gen/proto/ctrl/v1/ctrlv1connect/service.connect.go (1)
NewCtrlServiceHandler(85-101)go/gen/proto/ctrl/v1/ctrlv1connect/deployment.connect.go (1)
NewDeploymentServiceHandler(145-185)go/gen/proto/ctrl/v1/ctrlv1connect/openapi.connect.go (1)
NewOpenApiServiceHandler(86-102)go/gen/proto/ctrl/v1/ctrlv1connect/acme.connect.go (1)
NewAcmeServiceHandler(86-102)
go/cmd/deploy/main.go (1)
go/pkg/cli/flag.go (2)
String(376-408)EnvVar(287-304)
🪛 dotenv-linter (3.3.0)
apps/dashboard/.env.example
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The CTRL_API_KEY key should go before the CTRL_URL key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
QUICKSTART-DEPLOY.md
91-91: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Packages / Test
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (5)
go/cmd/deploy/main.go (2)
98-100: Good addition: carry API key through options.
282-284: Confirmed: ControlPlane client sets Authorization header using APIKey. NewControlPlaneClient / ControlPlaneClient set "Authorization: Bearer " from opts.APIKey (falls back to opts.AuthToken).go/apps/ctrl/middleware/auth.go (1)
54-86: Harden Authorization parsing; handle server misconfiguration.
- Parse Bearer scheme case-insensitively and tolerate extra spaces.
- Return a clear error if configured API key is empty.
Apply this diff:
- // Parse authorization header with case-insensitive Bearer scheme - const bearerScheme = "bearer" - if len(authHeader) < len(bearerScheme)+1 { - return nil, connect.NewError(connect.CodeUnauthenticated, - fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) - } - - // Extract scheme and check case-insensitively - schemePart := strings.ToLower(authHeader[:len(bearerScheme)]) - if schemePart != bearerScheme { - return nil, connect.NewError(connect.CodeUnauthenticated, - fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) - } - - // Ensure there's a space after the scheme - if authHeader[len(bearerScheme)] != ' ' { - return nil, connect.NewError(connect.CodeUnauthenticated, - fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) - } - - // Extract and trim the token - apiKey := strings.TrimSpace(authHeader[len(bearerScheme)+1:]) + // Parse authorization header with case-insensitive Bearer scheme; tolerate extra spaces + parts := strings.Fields(authHeader) + if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") { + return nil, connect.NewError(connect.CodeUnauthenticated, + fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) + } + apiKey := strings.TrimSpace(parts[1]) if apiKey == "" { return nil, connect.NewError(connect.CodeUnauthenticated, fmt.Errorf("API key cannot be empty")) } - // Simple API key validation against environment variable + // Simple API key validation against configured value // TODO: Replace with JWT validation when moving to private IP + if strings.TrimSpace(m.config.APIKey) == "" { + return nil, connect.NewError(connect.CodeUnauthenticated, + fmt.Errorf("server authentication not configured (CTRL_API_KEY missing)")) + } if apiKey != m.config.APIKey { return nil, connect.NewError(connect.CodeUnauthenticated, fmt.Errorf("Invalid API key")) }go/apps/ctrl/run.go (1)
207-223: Auth wiring looks good for the demo.Interceptor applied to all handlers; health/ACME exclusions handled in middleware. Log messages clarify behavior.
Consider toggling the interceptor off entirely when cfg.APIKey is empty if you prefer open local dev. If you want that, I can draft the change.
apps/dashboard/.env.example (1)
20-23: Sort Control Plane keys and remove quotes in .env.example
- In apps/dashboard/.env.example, list CTRL_API_KEY before CTRL_URL and remove the surrounding quotes from CTRL_API_KEY to satisfy dotenv-linter. Confirmed env.ts schema uses CTRL_URL and CTRL_API_KEY.
mcstepp
force-pushed
the
ENG-2082-ctrl-keys
branch
from
b4d9330 to
aa53752
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/local/src/cmd/dashboard.ts (1)
41-55: Don’t print secrets to the console.clack.note(env, …) will dump secrets (API keys, passwords, tokens). Mask sensitive values before displaying.
Apply this diff:
- clack.note(env, envPath); + // Mask secrets before displaying to the user + const masked = env.replace( + /^([A-Z0-9_]*(?:PASSWORD|TOKEN|SECRET|API_KEY))=.*/gmi, + "$1=***" + ); + clack.note(masked, envPath);
🧹 Nitpick comments (7)
apps/dashboard/.env.example (1)
21-22: Remove quotes from example API key.Other entries are unquoted, and keeping this one consistent avoids confusion for dotenv tooling that discourages quoting simple values (dotenv-linter already warns). Suggest:
-CTRL_API_KEY="your-local-dev-key" +CTRL_API_KEY=your-local-dev-keydeployment/docker-compose.yaml (1)
347-349: Avoid committing real secrets; prefer env files for local overrides.Place UNKEY_API_KEY in a .env (already ignored) and reference via env_file to reduce accidental leakage. Keep placeholders here.
go/cmd/deploy/main.go (1)
128-129: Consider making --api-key required for deploy, or document fallback precedence.To avoid 401s, either mark it required or define clear precedence between --api-key and --auth-token.
- cli.String("api-key", "API key for ctrl service authentication", cli.EnvVar("API_KEY")), + cli.String("api-key", "API key for ctrl service authentication", cli.EnvVar("API_KEY"), cli.Required()),go/apps/ctrl/middleware/auth.go (1)
76-81: Detect server misconfiguration (empty configured key).Fail fast if m.config.APIKey is empty to distinguish misconfig vs invalid client key.
- // Simple API key validation against environment variable + // Simple API key validation against configured value // TODO: Replace with JWT validation when moving to private IP + if strings.TrimSpace(m.config.APIKey) == "" { + return nil, connect.NewError(connect.CodeUnauthenticated, + fmt.Errorf("server authentication not configured (UNKEY_API_KEY missing)")) + } if apiKey != m.config.APIKey { return nil, connect.NewError(connect.CodeUnauthenticated, fmt.Errorf("Invalid API key")) }.gitignore (1)
27-27: Good ignore; consider broader pattern if multiple local DBs can appear.If metald can produce other sqlite/db files, consider a pattern (e.g., .db or metald.db) or scoping to a known path. Otherwise, this is fine as-is.
tools/local/src/cmd/dashboard.ts (1)
35-38: Avoid drift: source dashboard CTRL_API_KEY from the same secret used by ctrl.Use the ctrl service’s UNKEY_API_KEY (or existing CTRL_API_KEY) if available to keep a single source of truth; fall back to the placeholder only for first-time bootstrap.
Apply this diff:
ControlPlane: { CTRL_URL: "http://localhost:7091", - CTRL_API_KEY: "your-local-dev-key", + CTRL_API_KEY: + process.env.UNKEY_API_KEY ?? + process.env.CTRL_API_KEY ?? + "your-local-dev-key", },Based on learnings
apps/dashboard/lib/env.ts (1)
31-33: Constrain to non-empty string.Keep it optional but prevent empty strings from passing validation.
- CTRL_API_KEY: z.string().optional(), + CTRL_API_KEY: z.string().min(1).optional(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.gitignore(1 hunks)QUICKSTART-DEPLOY.md(2 hunks)apps/dashboard/.env.example(1 hunks)apps/dashboard/lib/env.ts(1 hunks)apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts(1 hunks)deployment/docker-compose.yaml(2 hunks)go/apps/ctrl/.gitignore(1 hunks)go/apps/ctrl/config.go(1 hunks)go/apps/ctrl/middleware/auth.go(1 hunks)go/apps/ctrl/run.go(2 hunks)go/cmd/ctrl/main.go(2 hunks)go/cmd/deploy/control_plane.go(3 hunks)go/cmd/deploy/main.go(3 hunks)tools/local/src/cmd/dashboard.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- go/apps/ctrl/.gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- go/cmd/deploy/control_plane.go
- go/apps/ctrl/config.go
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: go/cmd/deploy/control_plane.go:64-69
Timestamp: 2025-09-25T15:12:21.699Z
Learning: The API key authentication system in go/cmd/deploy/control_plane.go and related files is temporary demo code that will be replaced with a proper authentication system for private beta.
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
📚 Learning: 2025-09-24T18:57:34.843Z
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
Applied to files:
tools/local/src/cmd/dashboard.tsgo/cmd/ctrl/main.goapps/dashboard/.env.exampleapps/dashboard/lib/env.tsQUICKSTART-DEPLOY.mddeployment/docker-compose.yaml
📚 Learning: 2025-09-12T18:11:33.481Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/lib/trpc/routers/deploy/rollback.ts:23-24
Timestamp: 2025-09-12T18:11:33.481Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/rollback.ts, the CTRL_URL environment variable should fail fast with a clear error message if missing in non-development environments, rather than defaulting to localhost which can mask production configuration issues.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.tsQUICKSTART-DEPLOY.md
📚 Learning: 2025-07-25T19:11:00.208Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts:110-147
Timestamp: 2025-07-25T19:11:00.208Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/getOpenApiDiff.ts, the user mcstepp prefers to keep mock data fallbacks in POC/demonstration code for simplicity, even if it wouldn't be production-ready. This aligns with the PR being work-in-progress for demonstration purposes.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts
📚 Learning: 2025-09-25T15:12:21.699Z
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: go/cmd/deploy/control_plane.go:64-69
Timestamp: 2025-09-25T15:12:21.699Z
Learning: The API key authentication system in go/cmd/deploy/control_plane.go and related files is temporary demo code that will be replaced with a proper authentication system for private beta.
Applied to files:
go/cmd/ctrl/main.gogo/apps/ctrl/middleware/auth.gogo/cmd/deploy/main.go
📚 Learning: 2025-09-11T14:12:30.570Z
Learnt from: Flo4604
PR: unkeyed/unkey#3944
File: go/apps/ctrl/run.go:292-299
Timestamp: 2025-09-11T14:12:30.570Z
Learning: In go/apps/ctrl/config.go, the Validate() method already includes validation for Cloudflare API token using assert.NotEmpty() when both c.Acme.Enabled and c.Acme.Cloudflare.Enabled are true, with the error message "cloudflare API token is required when cloudflare is enabled".
Applied to files:
go/apps/ctrl/middleware/auth.go
📚 Learning: 2025-06-02T11:09:58.791Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3292
File: apps/dashboard/lib/trpc/routers/key/create.ts:11-14
Timestamp: 2025-06-02T11:09:58.791Z
Learning: In the unkey codebase, TypeScript and the env() function implementation already provide sufficient validation for environment variables, so additional runtime error handling for missing env vars is not needed.
Applied to files:
apps/dashboard/lib/env.ts
📚 Learning: 2025-07-22T09:02:12.495Z
Learnt from: chronark
PR: unkeyed/unkey#3638
File: deployment/docker-compose.yaml:81-94
Timestamp: 2025-07-22T09:02:12.495Z
Learning: The docker-compose.yaml file in deployment/ is specifically for development environments, not production. Kafka and other service configurations in this file should be optimized for development convenience rather than production security/hardening.
Applied to files:
QUICKSTART-DEPLOY.md
📚 Learning: 2025-07-15T14:45:18.920Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.920Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
Applied to files:
QUICKSTART-DEPLOY.md
📚 Learning: 2025-09-15T18:12:01.503Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: go/apps/ctrl/run.go:212-213
Timestamp: 2025-09-15T18:12:01.503Z
Learning: In Go connect-go generated handlers, mux.Handle can accept New*ServiceHandler functions directly even though they return (string, http.Handler) tuples. The pattern mux.Handle(ctrlv1connect.New*ServiceHandler(...)) is valid and compiles successfully in unkey codebase.
Applied to files:
go/apps/ctrl/run.go
📚 Learning: 2025-09-16T19:08:44.174Z
Learnt from: Flo4604
PR: unkeyed/unkey#3980
File: go/k8s/manifests/dashboard.yaml:41-42
Timestamp: 2025-09-16T19:08:44.174Z
Learning: For local development Kubernetes manifests (typically indicated by paths containing "local" or environment variables like UNKEY_REGION: "local"), hardcoded generic credentials in environment variables are acceptable for convenience. Security recommendations about using Secrets should be reserved for production or staging environments.
Applied to files:
deployment/docker-compose.yaml
🧬 Code graph analysis (4)
apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts (1)
apps/dashboard/lib/env.ts (1)
env(3-52)
go/cmd/ctrl/main.go (1)
go/pkg/cli/flag.go (3)
String(376-408)Required(267-284)EnvVar(287-304)
go/cmd/deploy/main.go (1)
go/pkg/cli/flag.go (2)
String(376-408)EnvVar(287-304)
go/apps/ctrl/run.go (5)
go/apps/ctrl/middleware/auth.go (2)
NewAuthMiddleware(24-28)AuthConfig(12-15)go/gen/proto/ctrl/v1/ctrlv1connect/service.connect.go (1)
NewCtrlServiceHandler(85-101)go/gen/proto/ctrl/v1/ctrlv1connect/deployment.connect.go (1)
NewDeploymentServiceHandler(145-185)go/gen/proto/ctrl/v1/ctrlv1connect/openapi.connect.go (1)
NewOpenApiServiceHandler(86-102)go/gen/proto/ctrl/v1/ctrlv1connect/acme.connect.go (1)
NewAcmeServiceHandler(86-102)
🪛 dotenv-linter (3.3.0)
apps/dashboard/.env.example
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The CTRL_API_KEY key should go before the CTRL_URL key
(UnorderedKey)
🪛 Gitleaks (8.28.0)
QUICKSTART-DEPLOY.md
[high] 138-138: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 markdownlint-cli2 (0.18.1)
QUICKSTART-DEPLOY.md
91-91: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Test Packages / Test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
QUICKSTART-DEPLOY.md (1)
14-28: Docs point to.env.local, but compose uses.env.
deployment/docker-compose.yamlmountsapps/dashboard/.envviaenv_file; editing.env.localwon’t affect the container, so the dashboard would still send the old default key and ctrl auth fails. Please align the docs with what compose actually reads (e.g., instruct users to editapps/dashboard/.env). As per coding guidelinesgo/cmd/ctrl/main.go (2)
57-58: Good: required API key flag wired to UNKEY_API_KEY.Required + env fallback matches the documented naming scheme for ctrl. Looks good for the demo.
Based on learnings
125-125: API key interceptor wired correctly cfg.APIKey is passed into NewAuthMiddleware and ConnectInterceptor is applied in go/apps/ctrl/run.go.go/cmd/deploy/main.go (2)
98-98: LGTM: DeployOptions includes APIKey.Matches CLI/env wiring and overall design.
211-212: Authorization header is set correctly
ControlPlaneClient usesc.opts.APIKey(falling back toAuthToken) and callsHeader().Set("Authorization", "Bearer "+authHeader)on both CreateDeployment and GetDeployment requests.deployment/docker-compose.yaml (1)
404-404: No action needed: dashboard reads CTRL_API_KEY server-side
apps/dashboard/lib/env.tsdefinesCTRL_API_KEYand tRPC routers useenv().CTRL_API_KEY; there is noNEXT_PUBLIC_CTRL_API_KEY.go/apps/ctrl/middleware/auth.go (1)
49-71: Harden Authorization parsing (tolerate extra whitespace; case-insensitive scheme).Current parsing requires exactly one space after scheme. Use SplitN/EqualFold.
- // Parse authorization header with case-insensitive Bearer scheme - const bearerScheme = "bearer" - if len(authHeader) < len(bearerScheme)+1 { - return nil, connect.NewError(connect.CodeUnauthenticated, - fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) - } - - // Extract scheme and check case-insensitively - schemePart := strings.ToLower(authHeader[:len(bearerScheme)]) - if schemePart != bearerScheme { - return nil, connect.NewError(connect.CodeUnauthenticated, - fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) - } - - // Ensure there's a space after the scheme - if authHeader[len(bearerScheme)] != ' ' { - return nil, connect.NewError(connect.CodeUnauthenticated, - fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) - } - - // Extract and trim the token - apiKey := strings.TrimSpace(authHeader[len(bearerScheme)+1:]) + // Parse authorization header with case-insensitive Bearer scheme + parts := strings.SplitN(authHeader, " ", 2) + if len(parts) != 2 || !strings.EqualFold(parts[0], "Bearer") { + return nil, connect.NewError(connect.CodeUnauthenticated, + fmt.Errorf("Invalid Authorization header format. Expected: Bearer <api_key>")) + } + // Extract and trim the token + apiKey := strings.TrimSpace(parts[1]) if apiKey == "" { return nil, connect.NewError(connect.CodeUnauthenticated, fmt.Errorf("API key cannot be empty")) }apps/dashboard/lib/env.ts (1)
31-33: CTRL_API_KEY remains server-only—noNEXT_PUBLIC_CTRL_*entries found and no references toCTRL_API_KEYin client components.
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (09/25/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
mcstepp
force-pushed
the
ENG-2082-ctrl-keys
branch
from
aa53752 to
9859c71
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
apps/dashboard/.env.example (1)
20-23: Unquote values and sort keys to satisfy dotenv-linter and avoid confusion.Remove quotes and place CTRL_API_KEY before CTRL_URL (per linter). This also matches our docs style elsewhere.
-# Control Plane -CTRL_URL=http://127.0.0.1:7091 -CTRL_API_KEY="your-local-dev-key" +# Control Plane +CTRL_API_KEY=your-local-dev-key +CTRL_URL=http://127.0.0.1:7091QUICKSTART-DEPLOY.md (5)
16-18: Unquote UNKEY_API_KEY in examples.Quotes in .env can trip linters and confuse readers. Use unquoted value.
-UNKEY_API_KEY="your-local-dev-key" +UNKEY_API_KEY=your-local-dev-key
91-98: Use headings instead of bold for “Option A/B” (markdownlint MD036).Switch to subheadings to satisfy lint and improve readability.
-**Option A: Environment variable (recommended)** +### Option A: Environment variable (recommended) @@ -**Option B: CLI flag** +### Option B: CLI flag
93-94: Avoid quoting the API key in export.Quotes are unnecessary in shell examples and we avoid normalizing to quoted values in docs.
-export API_KEY="your-local-dev-key" +export API_KEY=your-local-dev-key
102-109: Prefer env var over inline --api-key in examples.To reduce secret exposure and copy/paste risks, show the command without --api-key when Option A is used. If keeping the flag, avoid quotes.
go run . deploy \ --context=./demo_api \ --workspace-id="REPLACE_ME" \ --project-id="REPLACE_ME" \ --control-plane-url="http://127.0.0.1:7091" \ - --api-key="your-local-dev-key" \ + # --api-key=your-local-dev-key \ # Optional if not using API_KEY env var --keyspace-id="REPLACE_ME" # This is optional if you want key verifications
29-33: Optional: Document that compose already sets defaults for keys.If deployment/docker-compose.yaml ships sane defaults for UNKEY_API_KEY/CTRL_API_KEY, mention that Step 1 can be skipped unless overriding.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)QUICKSTART-DEPLOY.md(2 hunks)apps/dashboard/.env.example(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts:39-44
Timestamp: 2025-09-25T18:49:11.425Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts and similar files, mcstepp prefers to keep the demo API key authentication simple without additional validation complexity, since it's temporary code that will be replaced after the demo phase.
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: go/cmd/deploy/control_plane.go:64-69
Timestamp: 2025-09-25T15:12:21.699Z
Learning: The API key authentication system in go/cmd/deploy/control_plane.go and related files is temporary demo code that will be replaced with a proper authentication system for private beta.
📚 Learning: 2025-09-12T18:11:33.481Z
Learnt from: mcstepp
PR: unkeyed/unkey#3952
File: apps/dashboard/lib/trpc/routers/deploy/rollback.ts:23-24
Timestamp: 2025-09-12T18:11:33.481Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/rollback.ts, the CTRL_URL environment variable should fail fast with a clear error message if missing in non-development environments, rather than defaulting to localhost which can mask production configuration issues.
Applied to files:
QUICKSTART-DEPLOY.md
📚 Learning: 2025-09-24T18:57:34.843Z
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
Applied to files:
QUICKSTART-DEPLOY.mdapps/dashboard/.env.example
📚 Learning: 2025-07-22T09:02:12.495Z
Learnt from: chronark
PR: unkeyed/unkey#3638
File: deployment/docker-compose.yaml:81-94
Timestamp: 2025-07-22T09:02:12.495Z
Learning: The docker-compose.yaml file in deployment/ is specifically for development environments, not production. Kafka and other service configurations in this file should be optimized for development convenience rather than production security/hardening.
Applied to files:
QUICKSTART-DEPLOY.md
📚 Learning: 2025-07-15T14:45:18.920Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.920Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
Applied to files:
QUICKSTART-DEPLOY.md
📚 Learning: 2025-09-25T18:49:11.425Z
Learnt from: mcstepp
PR: unkeyed/unkey#4010
File: apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts:39-44
Timestamp: 2025-09-25T18:49:11.425Z
Learning: In apps/dashboard/lib/trpc/routers/deploy/deployment/rollback.ts and similar files, mcstepp prefers to keep the demo API key authentication simple without additional validation complexity, since it's temporary code that will be replaced after the demo phase.
Applied to files:
QUICKSTART-DEPLOY.md
🪛 Gitleaks (8.28.0)
QUICKSTART-DEPLOY.md
[high] 138-138: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 markdownlint-cli2 (0.18.1)
QUICKSTART-DEPLOY.md
91-91: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 dotenv-linter (3.3.0)
apps/dashboard/.env.example
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The CTRL_API_KEY key should go before the CTRL_URL key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build / Build
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
🔇 Additional comments (1)
apps/dashboard/.env.example (1)
20-23: No change needed for env variable names
apps/dashboard/lib/env.ts and docker-compose reference CTRL_URL and CTRL_API_KEY exclusively; no NEXT_PUBLIC_CTRL_* variables are used in the dashboard code.
3 participants
What does this PR do?
Implements simple API key authentication for both CLI and dashboard communication with thectrl service, and updates deployment documentation to reflect the new authentication requirements.
This PR introduces a basic API key authentication system for ctrl service communication:
go run . deployThe authentication flow:
CLI/Dashboard sends API_KEY/CTRL_API_KEY in requests → ctrl service validates against its configured CTRL_API_KEY → grants/denies access.
Fixes #3986
This addresses the need for secure communication between CLI/dashboard and ctrl service while maintaining simplicity for local development.
How To Test
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated