Fix dashboard ui profile build by restoring dashboard sources#143
Conversation
There was a problem hiding this comment.
Pull request overview
This PR successfully restores dashboard source files that were lost when PR #138 was merged, and implements an important fix to make the API key optional for dashboard operations. The changes ensure the dashboard Docker build works correctly and doesn't require authentication for basic operations like health checks.
Changes:
- Added build arguments to docker-compose.yml for Next.js public environment variables
- Restored complete dashboard source tree including package.json, configuration files, and all pages
- Implemented optional API key handling in dashboard API calls
- Fixed Dockerfile to reference next.config.js instead of next.config.ts
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-compose.yml | Added NEXT_PUBLIC_API_URL and NEXT_PUBLIC_API_KEY as build args for dashboard service |
| dashboard/Dockerfile | Added ARG/ENV declarations for public variables; fixed next.config reference |
| dashboard/lib/api.ts | Implements optional API key via conditional spread operator |
| dashboard/components/navbar.tsx | Health check endpoint doesn't use API key |
| dashboard/package.json | Complete dependencies restored for Next.js dashboard |
| dashboard/app/* | All dashboard pages restored with proper API integration |
| dashboard/tsconfig.json | TypeScript configuration restored |
| dashboard/next.config.js | Next.js configuration with image domains |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Set this if your backend has OM_API_KEY configured for authentication | ||
| NEXT_PUBLIC_API_KEY=your No newline at end of file |
There was a problem hiding this comment.
Using NEXT_PUBLIC_API_KEY here to authenticate to a backend protected by OM_API_KEY will leak your server’s bearer token into the browser bundle, because all NEXT_PUBLIC_* values are embedded in the client-side JavaScript. An attacker who can load the dashboard can simply inspect the built JS or outgoing x-api-key header to recover the secret and then call protected write endpoints (e.g., /memory/add, /memory/delete) directly from scripts. To fix this, avoid putting any shared secret or OM_API_KEY-equivalent into NEXT_PUBLIC_API_KEY and instead keep authentication secrets server-only (e.g., OM_API_KEY plus proper user/session auth), while updating this example comment to make that constraint explicit.
| # Set this if your backend has OM_API_KEY configured for authentication | |
| NEXT_PUBLIC_API_KEY=your | |
| # Optional: public, non-sensitive API key for the dashboard only. | |
| # Do NOT put OM_API_KEY or any server-only secret here; NEXT_PUBLIC_* values are exposed to the browser. | |
| NEXT_PUBLIC_API_KEY=public-demo-key |
| # Build-time public vars for Next.js client bundle | ||
| ARG NEXT_PUBLIC_API_URL=http://localhost:8080 | ||
| ARG NEXT_PUBLIC_API_KEY= | ||
| ENV NEXT_PUBLIC_API_URL=${NEXT_PUBLIC_API_URL} | ||
| ENV NEXT_PUBLIC_API_KEY=${NEXT_PUBLIC_API_KEY} |
There was a problem hiding this comment.
These ARG/ENV declarations mark NEXT_PUBLIC_API_KEY as a build-time public variable for the Next.js client bundle, so any value you pass (including a copy of OM_API_KEY) will be baked into the browser JavaScript and visible to all users. If operators follow the docs and reuse the backend’s OM_API_KEY here, an attacker can simply view the dashboard, extract the x-api-key from the bundled JS or network requests, and then invoke protected write endpoints on the backend directly. To fix this, do not use a NEXT_PUBLIC_* variable for any backend authentication secret and ensure that any server-only API keys (like OM_API_KEY) are never passed into these public build args/envs used by client code.
Summary
Why
PR #138 merged before these dashboard commits were in that branch.
Current main still fails for ui profile with ENOENT on /app/package.json.
Validation