app-server: add codex-device-key crate#18429
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69b06f874a
ℹ️ 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".
69b06f8 to
789f413
Compare
97a50d0 to
b857fcc
Compare
3f5129c to
6f55c87
Compare
b857fcc to
9a85c6c
Compare
d0ea927 to
7501a60
Compare
9a85c6c to
bf613e8
Compare
b8a5f17 to
e8bc757
Compare
bf613e8 to
9915c86
Compare
|
just make sure backend must verify signature against signed_payload_base64 bytes exactly. The crate now exposes exact bytes, but the security property is only real if the backend does NOT reserialize payloads. Please ensure backend verification uses the provided bytes and the token hash/nonce from the same challenge. |
9915c86 to
a38e6c1
Compare
e8bc757 to
4f7294e
Compare
|
[codex] Addressed in |
a38e6c1 to
3f76329
Compare
069064a to
4189212
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4189212c8a
ℹ️ 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".
| const REMOTE_CONTROL_CLIENT_PATHS: &[&str] = &[ | ||
| "/api/codex/remote/control/client", | ||
| "/api/codex/remote/control/client/enroll", | ||
| "/wham/remote/control/client", | ||
| "/wham/remote/control/client/enroll", |
There was a problem hiding this comment.
Split targetPath allowlists by payload audience
REMOTE_CONTROL_CLIENT_PATHS mixes /client and /client/enroll, and both payload validators call validate_remote_control_target unchanged. As a result, RemoteControlClientConnection accepts enrollment paths and RemoteControlClientEnrollment accepts websocket paths, violating each payload type’s route binding. This can permit cross-endpoint proof reuse if downstream verification assumes this crate enforces the path contract.
Useful? React with 👍 / 👎.
|
Non-blocking hardening note: validate key_id shape more strictly before using it downstream. Created ids are safe random dk_... values, but sign/get_public currently only reject empty key ids; the macOS provider later uses the caller-provided key_id in Keychain tags and binding metadata paths. Requiring exactly dk_ + unpadded base64url(32 bytes) would make this namespace auditable and remove path/tag weirdness. |
4189212 to
41e132c
Compare
41e132c to
e6e34ae
Compare
3f76329 to
bd1ef3b
Compare
e6e34ae to
ed32f55
Compare
## Why Device-key storage and signing are local security-sensitive operations with platform-specific behavior. Keeping the core API in codex-device-key keeps app-server focused on routing and business logic instead of owning key-management details. The crate also keeps the signing surface intentionally narrow: callers can create a bound key, fetch its public key, or sign one of the structured payloads accepted by the crate. It does not expose a generic arbitrary-byte signing API. Key IDs cross into platform-specific labels, tags, and metadata paths, so the crate constrains externally supplied IDs to the same auditable namespace it creates: dk_ plus unpadded base64url for 32 bytes. Remote-control target paths are tied to each signed payload shape so connection proofs cannot be reused for enrollment endpoints, or vice versa. ## What changed - Added the codex-device-key workspace crate. - Added account/client-bound key creation with stable dk_ key IDs. - Added strict key_id validation before public-key lookup or signing reaches a provider. - Added public-key lookup and structured signing APIs. - Split remote-control client endpoint allowlists by connection vs enrollment payload shape. - Added validation for key bindings, accepted payload fields, token expiration, and payload/key binding mismatches. - Added flow-oriented docs on the validation helpers that gate provider signing. - Added protection policy and protection-class types without wiring a platform provider yet. - Added an unsupported default provider so platforms without an implementation fail explicitly instead of silently falling back to software-backed keys. - Updated Cargo and Bazel lock metadata for the new crate and non-platform-specific dependencies. ## Validation - just fmt - cargo test -p codex-device-key - just fix -p codex-device-key - git diff --check
ed32f55 to
3661fcf
Compare
Why
Device-key storage and signing are local security-sensitive operations with platform-specific behavior. Keeping the core API in
codex-device-keykeeps app-server focused on routing and business logic instead of owning key-management details.The crate keeps the signing surface intentionally narrow: callers can create a bound key, fetch its public key, or sign one of the structured payloads accepted by the crate. It does not expose a generic arbitrary-byte signing API.
Key IDs cross into platform-specific labels, tags, and metadata paths, so externally supplied IDs are constrained to the same auditable namespace created by the crate:
dk_followed by unpadded base64url for 32 bytes. Remote-control target paths are also tied to each signed payload shape so connection proofs cannot be reused for enrollment endpoints, or vice versa.What changed
codex-device-keyworkspace crate.dk_key IDs.key_idvalidation before public-key lookup or signing reaches a provider.Stack
This is stacked on #18428.
Validation
cargo test -p codex-device-keykey_idvalidation before provider use.just bazel-lock-updatejust bazel-lock-check