Skip to content

app-server: implement device key v2 methods#18430

Merged
euroelessar merged 1 commit intomainfrom
ruslan/device-key-app-server
Apr 21, 2026
Merged

app-server: implement device key v2 methods#18430
euroelessar merged 1 commit intomainfrom
ruslan/device-key-app-server

Conversation

@euroelessar
Copy link
Copy Markdown
Collaborator

Why

The device-key protocol needs an app-server implementation that keeps local key operations behind the same request-processing boundary as other v2 APIs.

app-server owns request dispatch, transport policy, documentation, and JSON-RPC error shaping. codex-device-key owns key binding, validation, platform provider selection, and signing mechanics. Keeping the adapter thin makes the boundary easier to review and avoids moving local key-management details into thread orchestration code.

What changed

  • Added DeviceKeyApi as the app-server adapter around DeviceKeyStore.
  • Converted protocol protection policies, payload variants, algorithms, and protection classes to and from the device-key crate types.
  • Encoded SPKI public keys and DER signatures as base64 protocol fields.
  • Routed device/key/create, device/key/public, and device/key/sign through MessageProcessor.
  • Rejected remote transports before provider access while allowing local stdio and in-process callers to reach the device-key API.
  • Added stdio, in-process, and websocket tests for device-key validation and transport policy.
  • Documented the device-key methods in the app-server v2 method list.

Test coverage

  • device_key_create_rejects_empty_account_user_id
  • in_process_allows_device_key_requests_to_reach_device_key_api
  • device_key_methods_are_rejected_over_websocket

Stack

This is PR 3 of 4 in the device-key app-server stack. It is stacked on #18429.

Validation

  • cargo test -p codex-app-server device_key
  • just fix -p codex-app-server

@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from a6f5f5e to e4e9f03 Compare April 18, 2026 00:05
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from 69b06f8 to 789f413 Compare April 18, 2026 00:05
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from e4e9f03 to f9220dd Compare April 18, 2026 00:07
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from 789f413 to 4351eda Compare April 18, 2026 00:07
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from f9220dd to 75ba150 Compare April 18, 2026 00:15
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch 2 times, most recently from 3f5129c to 6f55c87 Compare April 18, 2026 00:22
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch 2 times, most recently from b59b13f to f7b198b Compare April 18, 2026 00:24
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch 2 times, most recently from 9b550dc to 6951f1a Compare April 18, 2026 00:26
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch 2 times, most recently from 54f7030 to c4e3b95 Compare April 18, 2026 00:35
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch 2 times, most recently from d0ea927 to 7501a60 Compare April 18, 2026 00:40
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from c4e3b95 to 20c2078 Compare April 18, 2026 00:40
Comment thread codex-rs/app-server/src/message_processor.rs Outdated
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from 20c2078 to cd244b7 Compare April 18, 2026 01:10
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch 2 times, most recently from b8a5f17 to e8bc757 Compare April 18, 2026 01:25
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from cd244b7 to b371887 Compare April 18, 2026 01:25
@viyatb-oai
Copy link
Copy Markdown
Collaborator

viyatb-oai commented Apr 18, 2026

[P1] Add regression coverage that device/key/* is rejected over remote‑control origin (not just raw websocket). Code blocks it via ConnectionOrigin::RemoteControl, but no test asserts the slingshot path can’t call device/key/sign.

@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from b371887 to f664fa1 Compare April 18, 2026 02:36
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from e8bc757 to 4f7294e Compare April 18, 2026 02:36
@euroelessar
Copy link
Copy Markdown
Collaborator Author

[codex] Addressed in f664fa1: added regression coverage for ConnectionOrigin::RemoteControl attempting device/key/sign through the app-server message processor. The test asserts the request is rejected with INVALID_REQUEST and the remote-transport rejection message.

@euroelessar euroelessar marked this pull request as draft April 18, 2026 02:38
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from f664fa1 to 123af8e Compare April 18, 2026 19:25
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch 2 times, most recently from 069064a to 4189212 Compare April 18, 2026 19:31
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from 123af8e to 03de8e3 Compare April 18, 2026 19:31
@euroelessar euroelessar marked this pull request as ready for review April 20, 2026 21:15
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from 4189212 to 41e132c Compare April 20, 2026 21:35
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from 03de8e3 to a2c2733 Compare April 20, 2026 21:35
@euroelessar euroelessar marked this pull request as draft April 20, 2026 21:39
@euroelessar euroelessar marked this pull request as ready for review April 20, 2026 21:49
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

pub(crate) fn default_provider() -> Arc<dyn DeviceKeyProvider> {
Arc::new(UnsupportedDeviceKeyProvider)
}
#[derive(Debug)]
pub(crate) struct UnsupportedDeviceKeyProvider;
impl DeviceKeyProvider for UnsupportedDeviceKeyProvider {
fn create(&self, request: ProviderCreateRequest<'_>) -> Result<DeviceKeyInfo, DeviceKeyError> {
let _ = request.key_id;
let _ = request
.protection_policy
.allows(DeviceKeyProtectionClass::HardwareTpm);
let _ = request.binding;
Err(DeviceKeyError::HardwareBackedKeysUnavailable)

P1 Badge Back device-key API with a real provider

device/key/* now routes through DeviceKeyStore::default(), but platform::default_provider() still returns UnsupportedDeviceKeyProvider. Its create always errors with HardwareBackedKeysUnavailable, and get_public/sign always return KeyNotFound, so the newly exposed app-server methods cannot succeed on any platform.

ℹ️ 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".

Copy link
Copy Markdown
Collaborator

@etraut-openai etraut-openai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One recommended addition for future maintainability, but otherwise looks good.


impl ConnectionOrigin {
pub(crate) fn allows_device_key_requests(self) -> bool {
matches!(self, Self::Stdio | Self::InProcess)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend adding a big comment here so it's clear to future reviewers (and more importantly to Codex!) that it's important to restrict this to local connection origins. That way, if a new remote connection origin is added, it will be properly excluded from this check.

@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from a2c2733 to 32d677d Compare April 21, 2026 07:43
@euroelessar euroelessar marked this pull request as draft April 21, 2026 07:44
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from 32d677d to 5212b20 Compare April 21, 2026 08:00
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from 41e132c to e6e34ae Compare April 21, 2026 08:00
@euroelessar euroelessar marked this pull request as ready for review April 21, 2026 08:01
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from e6e34ae to ed32f55 Compare April 21, 2026 08:39
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch 2 times, most recently from 5e7948e to ced4291 Compare April 21, 2026 17:16
@euroelessar euroelessar force-pushed the ruslan/device-key-crate branch from ed32f55 to 3661fcf Compare April 21, 2026 17:16
Base automatically changed from ruslan/device-key-crate to main April 21, 2026 17:57
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from ced4291 to f6a41e2 Compare April 21, 2026 18:50
@euroelessar euroelessar enabled auto-merge (squash) April 21, 2026 18:53
The protocol needs an app-server implementation that keeps local key operations behind the same request-processing boundary as other v2 APIs.

app-server owns request dispatch, transport policy, documentation, and JSON-RPC error shaping. `codex-device-key` owns key binding, validation, platform provider selection, and signing mechanics. Keeping that adapter thin makes the boundary easier to review and avoids moving local key-management details into thread orchestration code.

- Added `DeviceKeyApi` as the app-server adapter around `DeviceKeyStore`.
- Converted protocol protection policies, payload variants, algorithms, and protection classes to and from the device-key crate types.
- Encoded SPKI public keys and DER signatures as base64 protocol fields.
- Routed `device/key/create`, `device/key/public`, and `device/key/sign` through `MessageProcessor`.
- Rejected remote transports before provider access while allowing local `stdio` and in-process callers to reach the device-key API.
- Added stdio, in-process, and websocket tests for device-key validation and transport policy.
- Documented the device-key methods in the app-server v2 method list.

- `device_key_create_rejects_empty_account_user_id`
- `in_process_allows_device_key_requests_to_reach_device_key_api`
- `device_key_methods_are_rejected_over_websocket`

- `cargo test -p codex-app-server device_key`
- `just fix -p codex-app-server`
@euroelessar euroelessar force-pushed the ruslan/device-key-app-server branch from f6a41e2 to 6aeae28 Compare April 21, 2026 20:18
@euroelessar euroelessar disabled auto-merge April 21, 2026 21:07
@euroelessar euroelessar merged commit 69c3d12 into main Apr 21, 2026
37 of 39 checks passed
@euroelessar euroelessar deleted the ruslan/device-key-app-server branch April 21, 2026 21:07
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants