Skip to content

fix: null user_id for env bootstrap keys + H2 support for registry fetches#2370

Merged
kmendell merged 8 commits intogetarcaneapp:mainfrom
GiulioSavini:fix/swarm-agent-token-http2-registry
Apr 20, 2026
Merged

fix: null user_id for env bootstrap keys + H2 support for registry fetches#2370
kmendell merged 8 commits intogetarcaneapp:mainfrom
GiulioSavini:fix/swarm-agent-token-http2-registry

Conversation

@GiulioSavini
Copy link
Copy Markdown
Contributor

@GiulioSavini GiulioSavini commented Apr 13, 2026

Summary

  • 🐞 Bug: Deploying node level swarm agent fails on token create #2369 — Deploying a swarm node-level agent failed with ERROR: insert or update on table "api_keys" violates foreign key constraint "api_keys_user_id_fkey". When the agent processes GetNodeAgentDeployment it authenticates via the synthetic agentSudo middleware user (id = "agent"), which doesn't exist in the users table. Environment bootstrap API keys belong to the system, not a user — user_id is now nullable for these keys (migration 046, Postgres + SQLite). CreateEnvironmentApiKey passes nil; GetUserByApiKey rejects nil-user keys, which is correct since those keys are used for env pairing, not user authentication.

  • 🐞 Bug: Failed to fetch remote template registries due to malformed HTTP response HTTP/2 vs HTTP/1.x mismatch #2367 — Fetching remote template registries (including registry.getarcane.app and private Gitea instances) failed with net/http: HTTP/1.x transport connection broken: malformed HTTP response. Root cause: the custom *http.Transport created in httpx/client.go didn't set ForceAttemptHTTP2: true. Go conservatively disables HTTP/2 negotiation on any transport that has a custom DialContext; the safe-HTTP wrapper adds one, so H2 was silently dropped. Adding ForceAttemptHTTP2: true to both transport constructors re-enables ALPN negotiation for HTTPS connections.

Test plan

  • Existing TestEnvironmentService_EnsureSwarmNodeAgentEnvironment_* passes with updated nil-UserID assertion
  • go test ./internal/services/... ./pkg/utils/httpx/... ./internal/huma/... — all green
  • Deploy swarm node agent from agent-side UI — no FK error, install command generated
  • Toggle template registries on/off — community registry and Gitea registries fetch successfully
  • Roll back migration 046 (down.sql) — env bootstrap keys with NULL user_id are deleted cleanly

Closes #2369
Closes #2367

Disclaimer Greptiles Reviews use AI, make sure to check over its work.

To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike

To have Greptile Re-Review the changes, mention greptileai.

Greptile Summary

This PR fixes two production bugs: a foreign-key violation when swarm node agent environments create bootstrap API keys (by making user_id nullable via migration 046 for both Postgres and SQLite), and broken HTTP/2 negotiation for registry fetches (by adding ForceAttemptHTTP2: true to both transport constructors in httpx/client.go). The model, DTO, service logic, and tests are all updated consistently. Both fixes are correct and well-scoped.

Confidence Score: 5/5

Safe to merge; both bug fixes are correct and all remaining findings are P2 style issues.

No P0/P1 issues found. The nullable user_id migration, the nil guard in ValidateApiKey, and the ForceAttemptHTTP2 additions are all correct. The two P2 findings (dead userID parameter and markApiKeyUsedAsync ordering) do not affect correctness or security.

No files require special attention; api_key_service.go has two minor P2 style issues.

Comments Outside Diff (1)

  1. backend/internal/services/api_key_service.go, line 308 (link)

    P2 Dead userID parameter still accepted

    userID is accepted but silently ignored — the function now always passes nil to createAPIKeyWithRawKey. All three call-sites (environments.go:426, environments.go:578, environment_service.go:297) still forward a real user.ID, so developers reading those call-sites will reasonably believe the user is recorded on the key. Removing the parameter makes the contract explicit.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: backend/internal/services/api_key_service.go
    Line: 308
    
    Comment:
    **Dead `userID` parameter still accepted**
    
    `userID` is accepted but silently ignored — the function now always passes `nil` to `createAPIKeyWithRawKey`. All three call-sites (`environments.go:426`, `environments.go:578`, `environment_service.go:297`) still forward a real `user.ID`, so developers reading those call-sites will reasonably believe the user is recorded on the key. Removing the parameter makes the contract explicit.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Codex

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: backend/internal/services/api_key_service.go
Line: 308

Comment:
**Dead `userID` parameter still accepted**

`userID` is accepted but silently ignored — the function now always passes `nil` to `createAPIKeyWithRawKey`. All three call-sites (`environments.go:426`, `environments.go:578`, `environment_service.go:297`) still forward a real `user.ID`, so developers reading those call-sites will reasonably believe the user is recorded on the key. Removing the parameter makes the contract explicit.

```suggestion
func (s *ApiKeyService) CreateEnvironmentApiKey(ctx context.Context, environmentID string) (*apikey.ApiKeyCreatedDto, error) {
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: backend/internal/services/api_key_service.go
Line: 452-457

Comment:
**`markApiKeyUsedAsync` fires before the nil-UserID guard**

`markApiKeyUsedAsync` is called before the `UserID == nil` check. If an environment bootstrap key is accidentally presented to `ValidateApiKey`, its `last_used_at` gets written to the DB even though the function returns `ErrApiKeyInvalid` immediately after. Moving the guard above the async mark keeps `last_used_at` meaningful (only set on keys that actually complete authentication).

```suggestion
			if apiKey.UserID == nil {
				return nil, ErrApiKeyInvalid
			}

			s.markApiKeyUsedAsync(ctx, apiKey.ID)

```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix: allow null user_id on env bootstrap..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

… fetches

Two bugs in one branch:

1. Deploying a swarm node-level agent from the agent itself failed with a
   FK violation because CreateEnvironmentApiKey was inserting user_id='agent'
   (the synthetic agent user) which doesn't exist in the users table.
   Environment bootstrap keys belong to the system, not a user — make user_id
   nullable in api_keys and pass nil when creating env keys (migration 046).

2. Fetching remote template registries over HTTPS broke with
   "net/http: HTTP/1.x transport connection broken: malformed HTTP response"
   because the custom http.Transport used throughout the app didn't have
   ForceAttemptHTTP2 set, so Go disabled H2 negotiation entirely when a
   custom DialContext was applied in the safe-HTTP wrapper. Add
   ForceAttemptHTTP2: true to both constructors in httpx/client.go.

Closes getarcaneapp#2369
Closes getarcaneapp#2367
@kmendell
Copy link
Copy Markdown
Member

kmendell commented Apr 13, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Comment thread backend/internal/services/api_key_service.go
@GiulioSavini
Copy link
Copy Markdown
Contributor Author

working on it

@github-actions
Copy link
Copy Markdown

This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed.

Giulio Savini added 2 commits April 16, 2026 14:03
ValidateApiKey now rejects keys with user_id=NULL (bootstrap keys), so
the proxy validator fell back to 401 when forwarding to an env whose
apiUrl resolved back to the manager itself (common in CI/self-hosted).

Fall through to GetEnvironmentByApiKey so bootstrap keys are still
accepted by the proxy middleware.
@kmendell kmendell enabled auto-merge (squash) April 20, 2026 02:42
@kmendell kmendell merged commit a50cd8c into getarcaneapp:main Apr 20, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants