feat: auth hard fail without ENGRAM_API_TOKEN (ADR-0001)#11
Conversation
Server now refuses to start without ENGRAM_API_TOKEN unless ENGRAM_AUTH_DISABLED=true is explicitly set. When auth is disabled, a periodic warning logs every 60 seconds. Added /api/version to exempt paths for unauthenticated plugin version detection.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughДобавлена поддержка ENGRAM_AUTH_DISABLED для отключения обязательной аутентификации, проверка наличия токена при старте сервиса с фатальным завершением при отсутствии токена, расширен список exempt-путей, и добавлен периодический ворнинг при отключённой аутентификации. Changes
Sequence Diagram(s)sequenceDiagram
participant Env as Env (OS)
participant Service as WorkerService
participant Logger as Logger
participant Middleware as AuthMiddleware
participant Client as Client
Env->>Service: read GetWorkerToken, ENGRAM_AUTH_DISABLED
alt token present OR auth enabled with token
Service->>Logger: start server
else no token AND auth not disabled
Service->>Logger: log fatal -> exit
else auth disabled
Service->>Logger: start server
Service->>Logger: spawn goroutine (60s warning)
end
Client->>Service: HTTP request
Service->>Middleware: check token / exempt paths
alt path is exempt (e.g., /api/version)
Middleware->>Service: allow request
else token valid or auth disabled
Middleware->>Service: allow request
else missing/invalid token
Middleware->>Client: 401 Unauthorized (or warning only if auth disabled)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security posture of the application by making authentication mandatory by default. It introduces a startup gate that prevents the server from running without an API token, unless authentication is explicitly disabled. This change aligns with the new Architectural Decision Record (ADR-0001) for mandatory authentication with an explicit opt-out, ensuring that instances are not inadvertently exposed without security. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 strengthens the authentication mechanism by making it mandatory by default. The server will now exit if ENGRAM_API_TOKEN is not set, unless ENGRAM_AUTH_DISABLED is explicitly set to true. When auth is disabled, a periodic warning is logged. My review focuses on code duplication and ensuring graceful shutdown of the new background task. I've identified duplicated logic for checking the auth-disabled flag and a missing WaitGroup tracking for the new goroutine, which could affect shutdown reliability. I've provided suggestions to address these points.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/worker/middleware.go`:
- Around line 112-113: The middleware currently sets enabled := token != "" so a
non-empty token overrides the ENGRAM_AUTH_DISABLED flag; change the logic that
constructs the auth middleware (the struct fields enabled and token in
internal/worker/middleware.go, likely in the NewAuthMiddleware/constructor) to
honor ENGRAM_AUTH_DISABLED explicitly: compute enabled as false when
ENGRAM_AUTH_DISABLED is set to "true" (or the same flag source used in
internal/worker/service.go), otherwise enabled = token != ""; update the place
that reads the env var so the middleware never requires auth if
ENGRAM_AUTH_DISABLED=true even when token is non-empty.
In `@internal/worker/service.go`:
- Around line 1958-1964: The auth token check (GetWorkerToken +
ENGRAM_AUTH_DISABLED) is happening after heavy async initialization starts; move
this validation earlier so NewService performs the "hard-fail at startup" before
calling initializeAsync. Specifically, in NewService locate the call to
initializeAsync and insert the token validation logic (using
config.GetWorkerToken() and checking ENGRAM_AUTH_DISABLED via
strings.EqualFold(strings.TrimSpace(os.Getenv("ENGRAM_AUTH_DISABLED")), "true"))
before initializeAsync is invoked; if token is missing and auth is not disabled,
call log.Fatal().Msg with the existing message to abort startup immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9171ff35-91b4-48d0-9f41-b66ec01ebb7e
📒 Files selected for processing (2)
internal/worker/middleware.gointernal/worker/service.go
|
@codex review |
…ED in middleware - Move auth startup gate into NewService() before go svc.initializeAsync() so the process fails fast without wasting resources on heavy initialization when no token is configured and ENGRAM_AUTH_DISABLED is not set. The Start() check remains as a secondary layer for belt-and-suspenders safety. - Fix TokenAuth constructor: enabled was computed as token != "", which ignored ENGRAM_AUTH_DISABLED=true when a token was present. Now computed as token != "" && !authDisabled so an explicit opt-out always wins. - Track the periodic auth-disabled warning goroutine in s.wg so graceful shutdown waits for it to exit cleanly instead of potentially racing.
First emulation playbook for engram, scoped to v6.0.0 auth-two-tier-tokens release surfaces (server, plugin install, CLI proxy, dashboard tokens). Resolves the MISSING verdict from the emulation-playbook skill before v6.0.0 release. Future releases extend the scenario list per /emulation-playbook --add. Customer-mode walkthrough on this commit produced verdict PASS_WITH_DEFERRED: - Build, FR-4 fail-fast (server side), FR-4 fail-fast (client side) all PASS - Two UX surprises noted (no --help handler, hardcoded keycard URL in FATAL message) — queued as low-effort follow-ups, not in scope of v6.0.0 - Three scenarios (live-stand happy path, dashboard /tokens, plugin install) deferred to user-driven walkthrough before tag — they do not exercise the v6 auth contract; they exercise unchanged infrastructure surfaces Run report: .agent/reports/emulation-playbook-run-2026-04-26.md (gitignored, working state).
BREAKING CHANGES — v6.0.0 auth model. * Two-tier token model. Daemon and plugin no longer accept the operator key on workstations. Each workstation reads `ENGRAM_TOKEN` — a per-workstation API token (worker keycard) issued via the dashboard `/tokens` page. Operator key (`ENGRAM_AUTH_ADMIN_TOKEN`) lives ONLY on the server host. * Plugin `.mcp.json` env rename: `ENGRAM_AUTH_ADMIN_TOKEN` → `ENGRAM_TOKEN`. No legacy fallback chain. * Daemon fail-fast on missing token (FR-4): clean exit 1 instead of silent graceful-degrade to `loom_*-only` (PR #203 regression class). * Issuance hardening: `POST/GET/DELETE /api/auth/tokens` and `/api/auth/tokens/{id}/stats` require an admin browser session cookie; bearer-token authentication (operator key OR worker keycard) → 403. * Single `auth.Validator` shared by HTTP middleware + gRPC interceptor — the symmetric validation contract that PR #203's regression broke. * Strict token shape gate: `engram_` + exactly 32 hex chars; fails closed before any DB / bcrypt work. * Whitelisted scope mapping: only `read-write` / `read-only` are accepted from the api_tokens.scope column; anything else is treated as data corruption. * Connection pool keys credential AND TLS CA: rotating ENGRAM_TLS_CA or switching keycards cannot ride a stale pooled connection. * Bootstrap: 1 critical test (`tests/critical/auth_two_tier_test.go`) proves gRPC + HTTP both accept the same valid keycard via the same validator chain. Resolves the project-wide MISSING_SUITE gate. * Production playbook (`docs/PRODUCTION-TESTING-PLAYBOOK.md`) bootstraps the customer-mode walkthrough required by rule #11. Three scenarios (S2.full, S3, S4) require live-stand validation before v6.0.0 tag. 17/17 review threads resolved. Critical suite verdict: PASS.
1 participant
Summary
log.FatalifENGRAM_API_TOKENis not set andENGRAM_AUTH_DISABLEDis nottrue/api/versionadded to exempt paths (needed for plugin version detection)ADR
ADR-0001: Mandatory Authentication with Explicit Opt-Out
Test plan
docker run engramwithoutENGRAM_API_TOKEN→ exit 1 with clear messagedocker run -e ENGRAM_AUTH_DISABLED=true engram→ starts with warningdocker run -e ENGRAM_API_TOKEN=test engram→ starts normally/health,/api/version,/api/readyaccessible without tokenSummary by CodeRabbit
Примечания к выпуску
Новые возможности
Улучшения