Repository navigation
feat(api): POST /trust/handshake — idempotent genie-host registration (D5 Group 1.1) - #556
Conversation
Group 1.1 of the omni-host-fingerprint-trust wish (D5 follow-up). Builds on the schema landed in #555. Endpoint ======== POST /api/v2/trust/handshake body: { pubkey, hostname, capabilities? } 201 + { data: host } on first registration 200 + { data: existingHost } on idempotent replay GET /api/v2/trust/hosts 200 + { items: [host, ...] } (active hosts only — revokedAt IS NULL) Idempotent replay does NOT mutate hostname or capabilities. Operator- driven changes go through `omni trust update / revoke` (Group 1.2). Auth: inherits the existing bearer-token middleware. A valid omni_sk_… is required to register a new host. Once Group 4's verification middleware lands, signed requests from already-registered hosts can ALSO authenticate, but the FIRST handshake always requires bearer auth — there's no other way to bootstrap trust for a brand-new host. Validation: pubkey must look like a base64url-encoded 32-byte ed25519 key (43 chars unpadded or 44 chars padded; alphabet `[A-Za-z0-9_-]`, optional `=`). The actual cryptographic validation (is this a valid curve point?) lives in Group 4's per-request verifier — a format gate here just keeps obvious garbage out of the table. What changed ============ packages/api/src/services/genie-hosts.ts (new) + GenieHostsService — register / findByPubkey / findById / listActive / touchLastSeen. + No crypto in this service — only stores already-validated keys. Signature verification lives in Group 4's middleware so the data path can be reviewed independently. packages/api/src/services/index.ts + Wire GenieHostsService into the Services container. packages/api/src/routes/v2/trust.ts (new) + POST /handshake (idempotent on pubkey). + GET /hosts (active list). packages/api/src/routes/v2/index.ts + Mount /trust under /api/v2. packages/api/src/routes/v2/__tests__/trust-handshake.test.ts (new) + 6 contract tests covering: malformed pubkey rejection (400), empty hostname rejection (400), first registration (201), idempotent replay returns existing record without mutation (200), padded base64url accepted, GET /trust/hosts list shape. What's NOT in (subsequent PRs) ============================== - omni trust list/get/update/revoke CLI → Group 1.2 - genie omni handshake command (genie-side keygen) → Group 2 - genie request signing → Group 3 - omni signature verification middleware → Group 4 (security review) - per-host scope enforcement → Group 5 - --require-genie-signature per-instance opt-in → Group 6 - docs + brain → Group 7 Tests: 6/6 in trust-handshake.test.ts; typecheck FULL TURBO; biome clean.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 201fd879e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| v2Routes.route('/keys', keysRoutes); // API key management | ||
| v2Routes.route('/context', contextRoutes); // Conversation context for turn-based agents | ||
| v2Routes.route('/turns', turnsRoutes); // Turn lifecycle for turn-based agents | ||
| v2Routes.route('/trust', trustRoutes); // Genie host fingerprint trust (omni-host-fingerprint-trust wish) |
There was a problem hiding this comment.
Mounting /trust here introduces new protected endpoints, but this commit does not add corresponding entries in SCOPE_MAP (packages/api/src/constants/scopes.ts), and that middleware is explicitly deny-by-default for unmapped routes. In environments using scoped keys (anything without *), both POST /api/v2/trust/handshake and GET /api/v2/trust/hosts will return 403, making the new feature unreachable despite valid bearer auth.
Useful? React with 👍 / 👎.
| const existing = await this.findByPubkey(input.pubkey); | ||
| if (existing) { | ||
| log.info('genie host handshake — idempotent reuse', { hostId: existing.id, hostname: existing.hostname }); | ||
| return existing; | ||
| } | ||
|
|
||
| const [created] = await this.db | ||
| .insert(genieHosts) |
There was a problem hiding this comment.
This read-then-insert sequence is not atomic: two simultaneous handshakes with the same pubkey can both observe no existing row, then race on insert, causing one request to hit the unique constraint and fail (mapped to 409) instead of returning the existing host. That violates the endpoint’s idempotency contract in concurrent first-registration scenarios; use an atomic upsert/conflict path and then fetch the winner row.
Useful? React with 👍 / 👎.
![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 introduces the GenieHostsService and associated routes to handle host fingerprint registration and trust management. The implementation includes an idempotent handshake endpoint and a service layer for managing host records. Feedback focuses on ensuring true idempotency by normalizing public keys (removing base64 padding) and addressing a potential race condition in the registration logic where concurrent requests could cause unique constraint violations.
| const log = createLogger('genie-hosts'); | ||
|
|
||
| export interface RegisterHostInput { | ||
| /** ed25519 public key in base64url. Must be 44 chars (non-padded base64url of 32 bytes). */ |
There was a problem hiding this comment.
The comment contains a contradiction: 32 bytes of data encoded in base64url results in 43 characters when unpadded. A 44-character string would include a padding character (=). Since the service should ideally store a canonical (unpadded) form to ensure idempotency regardless of how the client handles padding, this comment should be updated to reflect the correct length.
| /** ed25519 public key in base64url. Must be 44 chars (non-padded base64url of 32 bytes). */ | |
| /** ed25519 public key in base64url. Normalized to 43 chars (unpadded base64url of 32 bytes). */ |
| async register(input: RegisterHostInput): Promise<GenieHost> { | ||
| const existing = await this.findByPubkey(input.pubkey); | ||
| if (existing) { | ||
| log.info('genie host handshake — idempotent reuse', { hostId: existing.id, hostname: existing.hostname }); | ||
| return existing; | ||
| } | ||
|
|
||
| const [created] = await this.db | ||
| .insert(genieHosts) | ||
| .values({ | ||
| pubkey: input.pubkey, | ||
| hostname: input.hostname, | ||
| capabilities: input.capabilities ?? {}, | ||
| }) | ||
| .returning(); | ||
|
|
||
| if (!created) { | ||
| throw new Error('failed to insert genie host'); | ||
| } | ||
|
|
||
| log.info('genie host registered', { hostId: created.id, hostname: created.hostname }); | ||
| return created; | ||
| } |
There was a problem hiding this comment.
The register method has two issues:
- Race Condition: It uses a "check-then-insert" pattern. If two concurrent requests for the same
pubkeyarrive, both might pass the initial check and attempt to insert, resulting in a unique constraint violation (500 error) for one of them. - Normalization: It doesn't normalize the
pubkeybefore insertion. Since the API allows both padded and unpadded base64url strings, they would be treated as different keys by the database, breaking the idempotency invariant.
Wrapping the insert in a try-catch to handle the race and normalizing the key ensures true idempotency. The redundant findByPubkey check at the start can also be removed since the route already performs this check for status code branching.
async register(input: RegisterHostInput): Promise<GenieHost> {
const pubkey = input.pubkey.replace(/=/g, '');
try {
const [created] = await this.db
.insert(genieHosts)
.values({
pubkey,
hostname: input.hostname,
capabilities: input.capabilities ?? {},
})
.returning();
if (!created) {
throw new Error('failed to insert genie host');
}
log.info('genie host registered', { hostId: created.id, hostname: created.hostname });
return created;
} catch (err) {
const existing = await this.findByPubkey(pubkey);
if (existing) {
log.info('genie host handshake — idempotent reuse (race)', { hostId: existing.id, hostname: existing.hostname });
return existing;
}
throw err;
}
}| async findByPubkey(pubkey: string): Promise<GenieHost | null> { | ||
| const [row] = await this.db.select().from(genieHosts).where(eq(genieHosts.pubkey, pubkey)).limit(1); | ||
| return row ?? null; | ||
| } |
There was a problem hiding this comment.
The pubkey should be normalized (e.g., by removing base64 padding) before performing the lookup. This ensures that clients providing the same key with different padding styles are correctly identified as the same host.
| async findByPubkey(pubkey: string): Promise<GenieHost | null> { | |
| const [row] = await this.db.select().from(genieHosts).where(eq(genieHosts.pubkey, pubkey)).limit(1); | |
| return row ?? null; | |
| } | |
| async findByPubkey(pubkey: string): Promise<GenieHost | null> { | |
| const normalized = pubkey.replace(/=/g, ''); | |
| const [row] = await this.db.select().from(genieHosts).where(eq(genieHosts.pubkey, normalized)).limit(1); | |
| return row ?? null; | |
| } |
2 participants
Summary
Group 1.1 of the omni-host-fingerprint-trust wish (D5 follow-up). Builds on the schema landed in #555 to expose the registration endpoint that
genie omni handshakewill call from the genie side (Group 2).Endpoints
Idempotency invariant
Re-registering the same
pubkeyreturns the existing record unchanged. Hostname and capabilities are NOT mutated by handshake replays — operator-driven changes go throughomni trust update / revoke(Group 1.2). This keeps the audit story clean: every change to a host's metadata is an explicit operator action, not a side-effect of a re-handshake.Auth model
The endpoint inherits the existing bearer-token middleware. A valid
omni_sk_…is required to register a new host. Once Group 4's verification middleware lands, signed requests from already-registered hosts can ALSO authenticate, but the first handshake always requires bearer auth — there's no other way to bootstrap trust for a brand-new host.Validation
pubkeymust look like a base64url-encoded 32-byte ed25519 key (43 chars unpadded or 44 chars padded; alphabet[A-Za-z0-9_-], optional=). The actual cryptographic validation (is this a valid curve point?) lives in Group 4's per-request verifier — a format gate here just keeps obvious garbage out of the table.What changed
packages/api/src/services/genie-hosts.ts(new)GenieHostsServicewithregister / findByPubkey / findById / listActive / touchLastSeen. No crypto — only stores already-validated keys. Signature verification lives in Group 4's middleware so the data path can be reviewed independently.packages/api/src/services/index.tsGenieHostsServiceinto theServicescontainer.packages/api/src/routes/v2/trust.ts(new)POST /handshake(idempotent on pubkey) +GET /hosts(active list).packages/api/src/routes/v2/index.ts/trustunder/api/v2.packages/api/src/routes/v2/__tests__/trust-handshake.test.ts(new)Test plan
bun test packages/api/src/routes/v2/__tests__/trust-handshake.test.ts→ 6/6 passmake typecheck→ green (FULL TURBO)bunx biome checkon touched files → cleanWhat's NOT in (subsequent PRs)
omni trust list / get / update / revokeCLIgenie omni handshake(genie-side keygen)--require-genie-signatureper-instance opt-inContext
Tracked under the
omni-host-fingerprint-trustwish. Predecessor: #555 (the foundation schema, landed in this repo). Wish doc lives in the genie repo at.genie/wishes/omni-host-fingerprint-trust/WISH.md(filed in genie #1520).