refactor(api): remove RSA signature gate from initial setup flow#6162
refactor(api): remove RSA signature gate from initial setup flow#6162gustavosbarreto merged 2 commits intomasterfrom
Conversation
|
Claude finished @geovannewashington's task in 4m 13s —— View job Code Review CompleteReviewed 3 files. 2 inline issues posted (1 critical, 1 medium). SummaryThe core handler and service changes are clean — the signature gate removal and interface simplification look correct. No cross-repo impact was found in However, the PR is incomplete: the test file ( Additional notesSecurity consideration: The previous RSA signature served as proof-of-server-access — only someone who could run the CLI (and read the private key) could complete setup. The new "first-user-wins" model removes this requirement, meaning any network-reachable client can call If you've addressed the feedback and want a new review, tag |
| return c.NoContent(http.StatusBadRequest) | ||
| } | ||
|
|
||
| var req requests.Setup |
There was a problem hiding this comment.
critical: The tests in api/routes/setup_test.go were not updated to match this simplified handler. The test file still:
- Sends
?sign=valuequery parameters (lines 42, 49, 61, 75, 96) which are now silently ignored - Sets up
servicesMock.On("SetupVerify", ...)mock expectations (lines 69, 83, 104) for a method the handler no longer calls — these expectations go unfulfilled - Contains test cases for removed behavior: "fail to get the signature" and "fail to validate the signature"
The tests may still compile and even pass accidentally (e.g., empty body still returns 400 from Bind), but they give false confidence — they're testing phantom code paths rather than the actual simplified flow.
The test file needs to be rewritten to:
- Remove the
queriesfield and all?sign=valuereferences - Remove all
SetupVerifymock expectations - Remove the "fail to get the signature" and "fail to validate the signature" test cases entirely
- Add
servicesMock.AssertExpectations(t)alongside the existingenvMock.AssertExpectations(t)to catch stale mock registrations going forward
| @@ -22,7 +14,6 @@ const PrivateKeyPath = "/var/run/secrets/api_private_key" | |||
|
|
|||
| type SetupService interface { | |||
| Setup(ctx context.Context, req requests.Setup) error | |||
There was a problem hiding this comment.
medium: Two cleanup items left behind:
-
PrivateKeyPath(line 13) is now unused — its only consumer was the removedSetupVerifymethod. It should be removed to avoid implying this path is still operationally relevant. -
The auto-generated mock at
api/services/mocks/services.gostill contains aSetupVerifymethod (lines 1454–1470) that is no longer part of this interface. The mock should be regenerated (e.g.,go generate ./api/services/...) to stay in sync.
Remove SetupVerify and all associated cryptographic validation from the setup initialization process. Replace the previous signed URL mechanism with a first-user-wins model based solely on system initialization state. Simplify the setup endpoint by removing the sign query parameter requirement and delegating access control entirely to system.Setup. Keep backward compatibility of the setup script by converting it into a simple URL opener without signature generation.
3004649 to
2067c92
Compare
|
/review |
|
Claude finished @otavio's task in 6s —— View job Code Review CompleteReviewed 6 files. 1 inline issue posted (1 critical). SummaryThe backend changes (Go handler, service, mocks) are clean — the signature gate was removed correctly, the However, the PR is incomplete: the frontend and OpenAPI spec still enforce the removed If you've addressed the feedback and want a new review, tag |
|
/review |
|
Claude finished @gustavosbarreto's task in 5m 17s —— View job Code Review CompleteReviewed 13 files across code quality, security, testing, Go/TypeScript patterns, and architecture — no issues found. SummaryThe signature gate removal is clean and complete across backend, frontend (both Vue and React UIs), and OpenAPI spec:
If you push additional changes and want a new review, tag |
What changed
Removed the RSA-based signature requirement from the setup flow and simplified the initial setup process.
The /setup endpoint no longer requires a sign query parameter and no longer performs cryptographic validation.
The setup script was updated to remove signature generation and now directly opens (or prints) the setup URL.
Cleanup was performed across:
Why
The previous setup flow introduced unnecessary complexity for a one-time initialization process.
Most self-hosted systems use a first-user-wins model where initialization is controlled by a persistent system state instead of cryptographic verification.
How to test
docker compose down -vmake startcurl http://localhost/info | jqExpected: setup: false
curl http://localhost/info | jqExpected: setup: true
Fixes: #6154