feat(agent-language): multi-key support for agent identity#683
Draft
HexaField wants to merge 4 commits intocoasys:devfrom
Draft
feat(agent-language): multi-key support for agent identity#683HexaField wants to merge 4 commits intocoasys:devfrom
HexaField wants to merge 4 commits intocoasys:devfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds multi-key management: new DNA externs and integrity/schema types for adding, revoking, and validating authorised keys; adapter, client, GraphQL resolver and Agent types updated; and extensive integration tests for multi-key flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/App
participant Resolver as AgentResolver
participant Adapter as ExpressionAdapterImpl
participant DNA as DNA Zome (agent_store)
participant Storage as Holochain Storage
Client->>Resolver: agentAddAuthorisedKey(key, name, proof)
Resolver->>Adapter: addAuthorisedKey(did, key, name, proof)
Adapter->>DNA: call add_authorised_key({did,key,name,proof})
DNA->>DNA: verify authorising_key, signature, uniqueness
DNA->>Storage: persist updated AgentExpression (authorised_keys)
Storage-->>DNA: ok
DNA-->>Adapter: return updated AgentExpressionData
Adapter-->>Resolver: return signed expression / Agent payload
Resolver-->>Client: return Agent with authorisedKeys
sequenceDiagram
participant Client as Client/App
participant Resolver as AgentResolver
participant Adapter as ExpressionAdapterImpl
participant DNA as DNA Zome (agent_store)
participant Storage as Holochain Storage
Client->>Resolver: agentRevokeKey(key, reason?)
Resolver->>Adapter: revokeKey(did, key, revokedByKey, signature, timestamp, reason?)
Adapter->>DNA: call revoke_key({did,key,revoked_by_key,signature,timestamp,reason})
DNA->>DNA: verify revoker authorised, signature, key present
DNA->>Storage: move key to revoked_keys, persist AgentExpression
Storage-->>DNA: ok
DNA-->>Adapter: return updated AgentExpressionData
Adapter-->>Resolver: return Agent with updated authorised/revoked lists
Resolver-->>Client: return updated Agent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Implements multi-key agent identity support (Phases 1 & 2): Phase 1 - Data Model + Zome Updates: - Add AuthorisedKey, KeyAuthorisation, KeyRevocation structs to integrity zome - Extend AgentExpressionData with authorised_keys and revoked_keys (serde(default) for backward compat) - Add add_authorised_key, revoke_key, is_key_valid zome functions - Auto-populate root key from DID on create_agent_expression (migration path) Phase 2 - Expression Validation + Client API: - Add TypeScript types with @ObjectType()/@inputType() decorators - Add GraphQL mutations: agentAddAuthorisedKey, agentRevokeKey - Add GraphQL queries: agentAuthorisedKeys, agentRevokedKeys, agentIsKeyValid - Add AgentClient methods for all new operations - Update AGENT_SUBITEMS fragment to include authorisedKeys and revokedKeys - Expose new zome functions through expression adapter Closes coasys#660
- Root key auto-population from did:key DIDs - Adding/revoking authorised keys - Edge cases: duplicate keys, invalid authorising keys, revoked key reuse - is_key_valid correctness - Backward compatibility with old-format expressions
HexaField
force-pushed
the
feat/multi-key-agent-identity
branch
from
9e0c566 to
d4ac79c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/agent/AgentClient.ts (1)
131-139:⚠️ Potential issue | 🟠 Major
me()silently dropsauthorisedKeys/revokedKeysfrom the query result.
AGENT_SUBITEMSnow fetches both fields, but the method explicitly reconstructsAgentand only propagatesdid,perspective, anddirectMessageLanguage. The same omission is present inupdatePublicPerspective(lines 253–256) andupdateDirectMessageLanguage(lines 303–306).🐛 Proposed fix
let agentObject = new Agent(agent.did, agent.perspective); agentObject.directMessageLanguage = agent.directMessageLanguage; +agentObject.authorisedKeys = agent.authorisedKeys; +agentObject.revokedKeys = agent.revokedKeys; return agentObject;Apply the same two-line addition in
updatePublicPerspectiveandupdateDirectMessageLanguagewhere a newAgentobject is manually assembled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/agent/AgentClient.ts` around lines 131 - 139, The me() method (and similarly updatePublicPerspective and updateDirectMessageLanguage) reconstructs a new Agent but omits authorisedKeys and revokedKeys from the GraphQL response; update each of these places (AgentClient.me, updatePublicPerspective, updateDirectMessageLanguage) to copy agent.authorisedKeys and agent.revokedKeys onto the newly created Agent instance (e.g., set the new Agent object's authorisedKeys and revokedKeys properties from the returned agent) so the keys from AGENT_SUBITEMS are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs`:
- Around line 112-115: The new struct fields authorised_keys and revoked_keys
serialize to snake_case and won't match the TypeScript Agent fields; update
their serde attributes on the struct to include explicit renames to the
camelCase names used by the TS layer (e.g., add #[serde(rename =
"authorisedKeys")] to authorised_keys and #[serde(rename = "revokedKeys")] to
revoked_keys while keeping #[serde(default)] so msgpack output matches the
expected Agent schema.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`:
- Around line 71-136: The add_authorised_key (and likewise revoke_key) lacks an
ownership check allowing any node to mutate another agent's keys; call
agent_info()? to get the calling agent's AgentInfo and compare
agent_info.agent_initial_pubkey (or its DID-equivalent) against the DID being
modified (input.did) and return an Err if they differ; add this guard at the
start of add_authorised_key and revoke_key (before any state changes or
validations) so only the DID owner can add/revoke keys.
- Around line 17-23: The helper extract_key_from_did currently returns None for
any DID that doesn't start with "did:key:", causing authorised_keys to be
silently empty during create_agent_expression; update extract_key_from_did to
explicitly handle unsupported DID methods by either returning a Result (Err with
a clear message) or logging a warning before returning None, and ensure callers
(e.g., create_agent_expression) propagate or handle that error/warning so
migration doesn't silently drop keys; also apply the same change to the
duplicate implementation in the 28-42 range and document the assumption that
only did:key is supported or expand parsing to other DID methods if intended.
- Around line 115-120: In add_authorised_key and revoke_key you construct a new
AgentExpression named new_expression but incorrectly copy current.proof (making
the proof invalid for the new data); instead, after building the new
AgentExpression fields (author, timestamp, data) generate a fresh cryptographic
proof/signature over the new expression content and assign that to
new_expression.proof (don’t reuse current.proof). Locate the new_expression
creation in both functions and replace the proof clone with a call to your
signing routine (e.g., sign_agent_expression or whatever agent signing helper
exists) that serialises the new expression payload and returns a new valid proof
before committing the entry.
- Around line 76-101: add_authorised_key currently trusts input.proof.signature
without cryptographic verification; update add_authorised_key to verify
input.proof.signature was produced by input.proof.authorising_key over the
canonical payload (e.g., the operation type + input.key + any nonce/timestamp)
using the platform's signature verification primitive before accepting the new
authorised key, and return an Err if verification fails; apply the same change
to revoke_key (verify input.signature was produced by the claimed authorising
key over the expected revoke payload) and add corresponding entry validation
callbacks so invalid signatures are rejected at validation time as well.
In `@core/src/agent/Agent.ts`:
- Around line 35-47: KeyAuthorisationInput is dead code: update the GraphQL
mutation that creates authorised keys so the proof is not dropped by either (A)
adding a proof parameter of type KeyAuthorisationInput to the mutation signature
used by agentAddAuthorisedKey and the resolver in AgentResolver.ts and pass it
through into the DNA extern call add_authorised_key (which expects proof:
KeyAuthorisation), or (B) if you intentionally don't want a proof at the GraphQL
boundary, delete KeyAuthorisationInput to avoid misleading API surface; modify
the function signature of agentAddAuthorisedKey and the resolver to match
whichever choice you make so the proof is either accepted and forwarded or the
input type removed everywhere.
In `@core/src/agent/AgentClient.ts`:
- Around line 309-335: The GraphQL mutations and client invocations for
agentAddAuthorisedKey and agentRevokeKey are missing required cryptographic
parameters so DNA calls will fail; update the GraphQL mutation signatures and
the client calls to accept and pass the KeyAuthorisationInput (proof with
authorisingKey and signature) into agentAddAuthorisedKey and to accept/pass the
signature into agentRevokeKey (or alternatively generate/inject the required
signature in the resolver prior to calling the adapter), and ensure the resolver
calls the adapter (not return dummy data) so the adapter receives these
parameters; locate references to agentAddAuthorisedKey, agentRevokeKey,
AGENT_SUBITEMS and the resolver implementation to make these changes.
In `@core/src/agent/AgentResolver.ts`:
- Around line 267-327: The test-only stubs for agentAddAuthorisedKey,
agentRevokeKey, agentAuthorisedKeys, agentRevokedKeys and agentIsKeyValid in
AgentResolver must be implemented in the real executor layer (Rust or JS)
instead of returning hardcoded values: replace the synthetic logic in
agentAddAuthorisedKey and agentRevokeKey with calls to the agent key management
service/storage to persist the authorised key and record revocations (including
timestamp, signer DID and signature), implement agentAuthorisedKeys and
agentRevokedKeys to read and return the stored lists, and implement
agentIsKeyValid to verify current validity by checking presence in authorised
keys and absence in revocation records (and signature/expiry checks if
applicable); ensure you expose the same resolver/method names
(agentAddAuthorisedKey, agentRevokeKey, agentAuthorisedKeys, agentRevokedKeys,
agentIsKeyValid) in the executor GraphQL layer and handle errors with
appropriate error responses.
---
Outside diff comments:
In `@core/src/agent/AgentClient.ts`:
- Around line 131-139: The me() method (and similarly updatePublicPerspective
and updateDirectMessageLanguage) reconstructs a new Agent but omits
authorisedKeys and revokedKeys from the GraphQL response; update each of these
places (AgentClient.me, updatePublicPerspective, updateDirectMessageLanguage) to
copy agent.authorisedKeys and agent.revokedKeys onto the newly created Agent
instance (e.g., set the new Agent object's authorisedKeys and revokedKeys
properties from the returned agent) so the keys from AGENT_SUBITEMS are
preserved.
bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs
Outdated
Show resolved
Hide resolved
bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs
Outdated
Show resolved
Hide resolved
bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs
Outdated
Show resolved
Hide resolved
bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs
Outdated
Show resolved
Hide resolved
Comment on lines
+267
to
+327
| @Mutation((returns) => Agent) | ||
| agentAddAuthorisedKey( | ||
| @Arg("key") key: string, | ||
| @Arg("name") name: string | ||
| ): Agent { | ||
| const agent = new Agent(TEST_AGENT_DID); | ||
| agent.authorisedKeys = [ | ||
| new AuthorisedKey( | ||
| key, | ||
| name, | ||
| new Date().toISOString(), | ||
| TEST_AGENT_DID, | ||
| new KeyAuthorisation(key, "test-signature") | ||
| ), | ||
| ]; | ||
| return agent; | ||
| } | ||
|
|
||
| @Mutation((returns) => Agent) | ||
| agentRevokeKey( | ||
| @Arg("key") key: string, | ||
| @Arg("reason", { nullable: true }) reason?: string | ||
| ): Agent { | ||
| const agent = new Agent(TEST_AGENT_DID); | ||
| agent.revokedKeys = [ | ||
| new KeyRevocation( | ||
| key, | ||
| new Date().toISOString(), | ||
| TEST_AGENT_DID, | ||
| "test-signature", | ||
| reason | ||
| ), | ||
| ]; | ||
| return agent; | ||
| } | ||
|
|
||
| @Query((returns) => [AuthorisedKey]) | ||
| agentAuthorisedKeys(): AuthorisedKey[] { | ||
| return [ | ||
| new AuthorisedKey( | ||
| "test-key", | ||
| "Root Key", | ||
| new Date().toISOString(), | ||
| TEST_AGENT_DID, | ||
| new KeyAuthorisation("test-key", "self") | ||
| ), | ||
| ]; | ||
| } | ||
|
|
||
| @Query((returns) => [KeyRevocation]) | ||
| agentRevokedKeys(): KeyRevocation[] { | ||
| return []; | ||
| } | ||
|
|
||
| @Query((returns) => Boolean) | ||
| agentIsKeyValid( | ||
| @Arg("did") did: string, | ||
| @Arg("key") key: string | ||
| ): Boolean { | ||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "agentAddAuthorisedKey\|agentRevokeKey\|agentAuthorisedKeys\|agentRevokedKeys\|agentIsKeyValid" --type=rs -C3Repository: coasys/ad4m
Length of output: 81
🏁 Script executed:
# Find rust files and search for the resolver names
find . -name "*.rs" -type f | head -20
rg "agentAddAuthorisedKey|agentRevokeKey|agentAuthorisedKeys|agentRevokedKeys|agentIsKeyValid" -C3 | head -100Repository: coasys/ad4m
Length of output: 7158
🏁 Script executed:
# Find rust-executor or executor-related files
find . -type d -name "*executor*" | head -20
find . -name "*resolver*" -o -name "*Resolver*" | grep -E "\.(ts|rs)$" | head -30Repository: coasys/ad4m
Length of output: 513
🏁 Script executed:
# Check mutation_resolvers.rs for agent key mutations
cat -n ./rust-executor/src/graphql/mutation_resolvers.rs | grep -A20 -B5 "agentAddAuthorisedKey\|agentRevokeKey\|agent_add_authorised_key\|agent_revoke_key"
# Check query_resolvers.rs for agent key queries
cat -n ./rust-executor/src/graphql/query_resolvers.rs | grep -A20 -B5 "agentAuthorisedKeys\|agentRevokedKeys\|agentIsKeyValid\|agent_authorised_keys\|agent_revoked_keys\|agent_is_key_valid"Repository: coasys/ad4m
Length of output: 37
🏁 Script executed:
# Check file sizes
wc -l ./rust-executor/src/graphql/mutation_resolvers.rs ./rust-executor/src/graphql/query_resolvers.rs
# Search for "agent" in both files (case-insensitive)
echo "=== mutation_resolvers.rs ==="
rg -i "agent" ./rust-executor/src/graphql/mutation_resolvers.rs -C2
echo "=== query_resolvers.rs ==="
rg -i "agent" ./rust-executor/src/graphql/query_resolvers.rs -C2Repository: coasys/ad4m
Length of output: 39609
🏁 Script executed:
# Check if the new resolvers exist anywhere else in the rust-executor
find ./rust-executor -name "*.rs" -type f | xargs grep -l "add_authorised_key\|revoke_key\|authorised_keys\|revoked_keys\|is_key_valid" 2>/dev/null
# Also check the graphql schema to see if these are defined
find . -name "*.graphql" -o -name "*.gql" | head -10Repository: coasys/ad4m
Length of output: 297
🏁 Script executed:
# Check the GraphQL schema to see how these resolvers are defined
echo "=== Checking agent.gql schema ==="
grep -i "authorised\|revoked\|isKeyValid" ./rust-client/src/agent.gql
echo "=== Checking runtime.gql schema ==="
grep -i "authorised\|revoked\|isKeyValid" ./rust-client/src/runtime.gql
# Check if there's any implementation pattern for how resolvers delegate to other code
grep -n "callResolver" ./rust-executor/src/graphql/mutation_resolvers.rs | head -5
grep -n "callResolver" ./rust-executor/src/graphql/query_resolvers.rs | head -5Repository: coasys/ad4m
Length of output: 1273
🏁 Script executed:
# Let's look at the actual agent schema more carefully
cat ./rust-client/src/agent.gql | head -100
# And check the TypeScript resolver file to understand the pattern
cat -n ./core/src/agent/AgentResolver.ts | head -80Repository: coasys/ad4m
Length of output: 3904
🏁 Script executed:
# Search the entire codebase for these resolver names to see if they're referenced
echo "=== Searching for agentAddAuthorisedKey ==="
rg "agentAddAuthorisedKey" --type ts --type js -C2 | head -50
echo "=== Searching for implementation patterns ==="
# Check if there's a schema file that defines these resolvers
find . -name "*.ts" -path "*/src/*" | xargs grep -l "agentAddAuthorisedKey\|agentRevokeKey\|agentAuthorisedKeys\|agentRevokedKeys\|agentIsKeyValid" 2>/dev/nullRepository: coasys/ad4m
Length of output: 1322
🏁 Script executed:
# Let's understand the architecture - what is the core directory?
echo "=== Directory structure ==="
ls -la ./core/
# Check if core is a test or development fixture
find ./core -name "package.json" | head -5
# Look for how these resolvers are wired in the actual application
find . -name "*.ts" -path "*/src/*" | xargs grep -l "AgentResolver" | head -10
# Check if there's a test-specific resolver file
ls -la ./core/src/agent/ | grep -i resolverRepository: coasys/ad4m
Length of output: 1400
🏁 Script executed:
# Check what core package is
cat ./core/package.json | head -40
# Look at the buildSchema to see how resolvers are wired
cat -n ./core/src/buildSchema.ts | head -60
# Check if there's a README that explains the architecture
head -50 ./core/README.mdRepository: coasys/ad4m
Length of output: 4905
🏁 Script executed:
# Look at the generated schema to see if these resolvers are in it
cat ./core/src/schema.gql | grep -i "agentAddAuthorisedKey\|agentRevokeKey\|agentAuthorisedKeys\|agentRevokedKeys\|agentIsKeyValid" -A5 -B5
# Check if there's any comment or documentation about these being test-only
grep -B10 "agentAddAuthorisedKey" ./core/src/agent/AgentResolver.ts | head -20Repository: coasys/ad4m
Length of output: 395
🏁 Script executed:
# Check the full AgentResolver.ts file to understand the context
wc -l ./core/src/agent/AgentResolver.ts
# Check the end of the file to see all resolver methods
tail -100 ./core/src/agent/AgentResolver.ts
# Search in rust-executor for any comment or pattern about delegating to TypeScript
grep -n "callResolver.*agent" ./rust-executor/src/graphql/mutation_resolvers.rs | head -10Repository: coasys/ad4m
Length of output: 2803
🏁 Script executed:
# Check if there's any delegation pattern we missed - search for where TypeScript resolvers are called from rust
grep -r "agentAddAuthorisedKey\|agentRevokeKey\|agentAuthorisedKeys\|agentRevokedKeys\|agentIsKeyValid" ./rust-executor/ 2>/dev/null
# Look at how AgentClient.ts is used - does it connect to rust-executor or the TypeScript stubs?
grep -n "apolloClient\|wsLink" ./core/src/agent/AgentClient.ts | head -5
# Check the executor directory to see if there's another resolver location
ls -la ./executor/
find ./executor -name "*resolver*" 2>/dev/nullRepository: coasys/ad4m
Length of output: 1138
🏁 Script executed:
# Check what's in executor/src
find ./executor/src -name "*resolver*" -o -name "*Resolver*" 2>/dev/null
# Check executor package.json to understand what it is
cat ./executor/package.json | head -50
# Look at executor/src structure
ls -la ./executor/src/Repository: coasys/ad4m
Length of output: 2744
🏁 Script executed:
# Check executor/src/core for resolver or GraphQL implementations
find ./executor/src/core -type f -name "*.ts" | head -20
# Check if executor has agent-related implementations
ls -la ./executor/src/core/
# Search executor for these resolver names
grep -r "agentAddAuthorisedKey\|agentRevokeKey\|agentAuthorisedKeys\|agentRevokedKeys\|agentIsKeyValid" ./executor/src/ 2>/dev/nullRepository: coasys/ad4m
Length of output: 1564
Implement real resolvers in rust-executor and/or executor for these new key management mutations and queries.
The five new resolvers are currently stub implementations in the test resolver only (core/src/agent/AgentResolver.ts). No real implementations exist in either rust-executor/src/graphql/ or executor/src/core/. These stubs hardcode test data and will become invisible to integration tests:
agentAddAuthorisedKeyandagentRevokeKeymutations return synthetic dataagentAuthorisedKeysalways returns one hardcoded test keyagentRevokedKeysalways returns an empty arrayagentIsKeyValidalways returnstrue
Real implementations must be added to the appropriate executor layer (either Rust or JavaScript) before this is shipped, or these operations will not function in production.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/agent/AgentResolver.ts` around lines 267 - 327, The test-only stubs
for agentAddAuthorisedKey, agentRevokeKey, agentAuthorisedKeys, agentRevokedKeys
and agentIsKeyValid in AgentResolver must be implemented in the real executor
layer (Rust or JS) instead of returning hardcoded values: replace the synthetic
logic in agentAddAuthorisedKey and agentRevokeKey with calls to the agent key
management service/storage to persist the authorised key and record revocations
(including timestamp, signer DID and signature), implement agentAuthorisedKeys
and agentRevokedKeys to read and return the stored lists, and implement
agentIsKeyValid to verify current validity by checking presence in authorised
keys and absence in revocation records (and signature/expiry checks if
applicable); ensure you expose the same resolver/method names
(agentAddAuthorisedKey, agentRevokeKey, agentAuthorisedKeys, agentRevokedKeys,
agentIsKeyValid) in the executor GraphQL layer and handle errors with
appropriate error responses.
Contributor
Author
|
Re: caller authorisation & revocation authority (#2 and #3 from the review) @lucksus — couple of questions on the right approach here: For enforcing that only holders of an existing authorised key can add/revoke keys, what's your preferred pattern in the agent language DNA?
Related: for revocation, should revoking a key also invalidate any I've added signature verification for the proofs in this PR (Ed25519 verify on add/revoke). The authorisation layer is the next piece — wanted your input before building it. ⬡ Hex |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/agent/Agent.ts (1)
5-33: 🛠️ Refactor suggestion | 🟠 MajorAgent class docstring is now misplaced above
KeyAuthorisation.The block comment at lines 5–20 ("AD4M's representation of an Agent …") was originally the JSDoc for the
Agentclass (line 101). Inserting the new types between the comment and the class causes the comment to appear as documentation forKeyAuthorisationinstead. Move it back to directly above@ObjectType() export class Agent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/agent/Agent.ts` around lines 5 - 33, The class-level JSDoc describing AD4M Agents was left above KeyAuthorisation and is now documenting the wrong class; move the entire comment block that begins "AD4M's representation of an Agent..." so it sits immediately above the `@ObjectType`() export class Agent declaration (not above class KeyAuthorisation), ensuring KeyAuthorisation keeps its own brief doc (or none) and Agent regains the original JSDoc; update only the comment placement, not the comment text or class code.
🧹 Nitpick comments (2)
bootstrap-languages/agent-language/adapter.ts (1)
41-41: Use nullish coalescing (??) instead of logical OR (||) forreason.
reason || nullconverts an empty string""tonull. If an empty-string reason is a valid (if unlikely) input,??preserves it correctly.Proposed fix
- { did, key, signature, reason: reason || null } + { did, key, signature, reason: reason ?? null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap-languages/agent-language/adapter.ts` at line 41, Replace the use of logical OR for the reason field so empty-string reasons are preserved: in the object construction where you set { did, key, signature, reason: reason || null } (in adapter.ts) change it to use nullish coalescing so the field becomes reason: reason ?? null; this ensures only undefined/null map to null while "" is kept as-is.bootstrap-languages/agent-language/hc-dna/tests/sweettest/src/test_multi_key.rs (1)
204-252: Consider adding a test case foris_key_validon a revoked key.The current test covers: authorised key →
true, unknown key →false, non-existent DID →false. It doesn't verify that a previously-authorised-then-revoked key returnsfalsefromis_key_valid. That's arguably the most important scenario for this function.test_revoke_key_successverifies list membership but not theis_key_validquery path after revocation.💡 Suggested additional assertion block (append after line 251)
// --- Add + revoke a key, then confirm is_key_valid returns false --- let add_input = AddAuthorisedKeyInput { did: did.to_string(), key: "ephemeral-key".to_string(), name: "Ephemeral".to_string(), proof: KeyAuthorisation { authorising_key: root_key.to_string(), signature: "sig".to_string(), }, }; let _: AgentExpression = call_zome(&conductor, &cell, "add_authorised_key", add_input).await; let revoke_input = RevokeKeyInput { did: did.to_string(), key: "ephemeral-key".to_string(), signature: "sig".to_string(), reason: None, }; let _: AgentExpression = call_zome(&conductor, &cell, "revoke_key", revoke_input).await; let valid: bool = call_zome( &conductor, &cell, "is_key_valid", IsKeyValidInput { did: did.to_string(), key: "ephemeral-key".to_string(), }, ) .await; assert!(!valid, "Revoked key should be invalid");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap-languages/agent-language/hc-dna/tests/sweettest/src/test_multi_key.rs` around lines 204 - 252, Add a test case that adds then revokes a key and asserts is_key_valid returns false: call the zome "add_authorised_key" with an AddAuthorisedKeyInput (use did, key "ephemeral-key", name and KeyAuthorisation with root_key as authorising_key), then call "revoke_key" with a RevokeKeyInput for "ephemeral-key", and finally call "is_key_valid" with IsKeyValidInput for that ephemeral key and assert!(!valid). Use the existing conductor/cell and types (AddAuthorisedKeyInput, RevokeKeyInput, IsKeyValidInput, KeyAuthorisation) and the zome entry points add_authorised_key, revoke_key, is_key_valid to locate where to insert the block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs`:
- Around line 59-81: The nested structs KeyAuthorisation, AuthorisedKey, and
KeyRevocation (and the input structs AddAuthorisedKeyInput, RevokeKeyInput,
IsKeyValidInput) need consistent camelCase serialization for TypeScript
interoperability; add #[serde(rename_all = "camelCase")] to each of those struct
definitions so fields like authorising_key, added_at, added_by, revoked_key,
revoked_at, revoked_by are serialized as authorisingKey, addedAt, addedBy,
revokedKey, revokedAt, revokedBy respectively.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`:
- Around line 70-188: Both add_authorised_key and revoke_key perform a
read-modify-write on the same DID without concurrency checks, causing lost
updates; add optimistic versioning or post-write validation: extend
AgentExpressionData with a version/seq field (e.g.,
AgentExpressionData::version), have get_current_expression return the version,
on mutation check that current.version matches the version we read, increment
the version on the new AgentExpression before creating the
EntryTypes::AgentExpression, and return an error (or retry) if the version has
changed; alternatively (or additionally) after create_entry of the new
EntryTypes::AgentExpression re-read get_current_expression and verify the latest
entry hash/timestamp/version matches the one you just wrote and abort/rollback
if it does not (apply these checks in both add_authorised_key and revoke_key,
referencing get_current_expression, AgentExpressionData::version, create_entry,
and EntryTypes::AgentExpression).
---
Outside diff comments:
In `@core/src/agent/Agent.ts`:
- Around line 5-33: The class-level JSDoc describing AD4M Agents was left above
KeyAuthorisation and is now documenting the wrong class; move the entire comment
block that begins "AD4M's representation of an Agent..." so it sits immediately
above the `@ObjectType`() export class Agent declaration (not above class
KeyAuthorisation), ensuring KeyAuthorisation keeps its own brief doc (or none)
and Agent regains the original JSDoc; update only the comment placement, not the
comment text or class code.
---
Duplicate comments:
In
`@bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs`:
- Around line 112-115: The struct fields authorised_keys and revoked_keys in
lib.rs are missing serde renames for the TypeScript camelCase names; update the
field attributes so authorised_keys uses #[serde(default, rename =
"authorisedKeys")] and revoked_keys uses #[serde(default, rename =
"revokedKeys")] (keeping the existing Vec types and defaults) so serialization
matches the Agent TypeScript shape.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`:
- Around line 16-23: The helper extract_key_from_did currently returns None
silently for non-"did:key:" inputs; change its behavior to return a
Result<String, String> (or a custom error) so failures surface: update fn
extract_key_from_did to return Err containing a clear message (including the
original did) when the prefix isn't "did:key:", adjust all callers (e.g., where
extract_key_from_did is invoked) to handle the Result (propagate or map the
error), and ensure logging/propagation uses the new error message instead of
silently ignoring the mismatch.
- Around line 71-136: The functions add_authorised_key and revoke_key are
missing caller-identity and signature checks; update add_authorised_key (and the
analogous revoke_key) to (1) verify the zome caller's AgentPubKey matches the
current expression owner (e.g. compare caller() / agent_info() to current.author
or input.did) to enforce ownership, and (2) cryptographically verify the
provided proof.signature (or proof field) using the claim that
proof.authorising_key is an authorised key in current.data.authorised_keys
before accepting changes; perform the same caller and signature validation
inside revoke_key, and abort with an error if either check fails so only the
legitimate owner holding a valid signature from an authorised key can add or
revoke keys.
- Around line 115-120: The new AgentExpression must not reuse
current.proof.clone() because that proof is invalid for the new data; update the
construction of new_expression (the AgentExpression struct) to generate a fresh
proof for the combination of author, timestamp (now) and new_data instead of
cloning current.proof — call the appropriate signing/proof function (e.g.,
sign_agent_expression / compute_proof / whatever helper exists in this module)
with author, now, and new_data and assign its result to proof; if no helper
exists, add a small helper to produce the cryptographic proof/signature for
AgentExpression and use it here rather than reusing current.proof.clone().
In `@core/src/agent/Agent.ts`:
- Around line 35-47: KeyAuthorisationInput is declared but never used because
the GraphQL mutation agentAddAuthorisedKey in AgentResolver only accepts key and
name, so the proof (signature/authorisingKey) is dropped; either remove
KeyAuthorisationInput or wire it through: update the AgentResolver mutation
signature to accept a KeyAuthorisationInput (or add an additional argument named
authorisationInput), propagate that input into the service call (e.g., the same
place agentAddAuthorisedKey calls into the Agent service or repository), and
ensure any verification logic (verifyAuthorisation, validateSignature, or
similar) consumes the signature and authorisingKey fields; also update any
GraphQL schema/types and tests accordingly so the proof is passed and validated
end-to-end.
---
Nitpick comments:
In `@bootstrap-languages/agent-language/adapter.ts`:
- Line 41: Replace the use of logical OR for the reason field so empty-string
reasons are preserved: in the object construction where you set { did, key,
signature, reason: reason || null } (in adapter.ts) change it to use nullish
coalescing so the field becomes reason: reason ?? null; this ensures only
undefined/null map to null while "" is kept as-is.
In
`@bootstrap-languages/agent-language/hc-dna/tests/sweettest/src/test_multi_key.rs`:
- Around line 204-252: Add a test case that adds then revokes a key and asserts
is_key_valid returns false: call the zome "add_authorised_key" with an
AddAuthorisedKeyInput (use did, key "ephemeral-key", name and KeyAuthorisation
with root_key as authorising_key), then call "revoke_key" with a RevokeKeyInput
for "ephemeral-key", and finally call "is_key_valid" with IsKeyValidInput for
that ephemeral key and assert!(!valid). Use the existing conductor/cell and
types (AddAuthorisedKeyInput, RevokeKeyInput, IsKeyValidInput, KeyAuthorisation)
and the zome entry points add_authorised_key, revoke_key, is_key_valid to locate
where to insert the block.
bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs
Show resolved
Hide resolved
Comment on lines
+70
to
+188
| #[hdk_extern] | ||
| pub fn add_authorised_key(input: AddAuthorisedKeyInput) -> ExternResult<AgentExpression> { | ||
| let current = get_current_expression(&input.did)? | ||
| .ok_or_else(|| err("Agent expression not found"))?; | ||
|
|
||
| // Check that the authorising key is in the current authorised_keys | ||
| let authorising_key_valid = current | ||
| .data | ||
| .authorised_keys | ||
| .iter() | ||
| .any(|k| k.key == input.proof.authorising_key); | ||
|
|
||
| if !authorising_key_valid { | ||
| return Err(err("Authorising key is not in the current authorised keys")); | ||
| } | ||
|
|
||
| // Check key is not already revoked | ||
| let is_revoked = current | ||
| .data | ||
| .revoked_keys | ||
| .iter() | ||
| .any(|r| r.revoked_key == input.proof.authorising_key); | ||
|
|
||
| if is_revoked { | ||
| return Err(err("Authorising key has been revoked")); | ||
| } | ||
|
|
||
| // Check the new key isn't already authorised | ||
| let already_exists = current.data.authorised_keys.iter().any(|k| k.key == input.key); | ||
| if already_exists { | ||
| return Err(err("Key is already authorised")); | ||
| } | ||
|
|
||
| let now = chrono::Utc::now(); | ||
| let new_key = AuthorisedKey { | ||
| key: input.key, | ||
| name: input.name, | ||
| added_at: now, | ||
| added_by: input.did.clone(), | ||
| proof: input.proof, | ||
| }; | ||
|
|
||
| let mut new_data = current.data.clone(); | ||
| new_data.authorised_keys.push(new_key); | ||
|
|
||
| let new_expression = AgentExpression { | ||
| author: current.author.clone(), | ||
| timestamp: now, | ||
| data: new_data, | ||
| proof: current.proof.clone(), | ||
| }; | ||
|
|
||
| // Store updated expression | ||
| let did = EntryTypes::Did(Did(current.author.clone())); | ||
| let did_hash = hash_entry(&did)?; | ||
| let entry = EntryTypes::AgentExpression(new_expression.clone()); | ||
| let entry_hash = hash_entry(&entry)?; | ||
| create_entry(&entry)?; | ||
| create_link( | ||
| did_hash, | ||
| entry_hash, | ||
| LinkTypes::ProfileLink, | ||
| LinkTag::new("profile"), | ||
| )?; | ||
|
|
||
| Ok(new_expression) | ||
| } | ||
|
|
||
| #[hdk_extern] | ||
| pub fn revoke_key(input: RevokeKeyInput) -> ExternResult<AgentExpression> { | ||
| let current = get_current_expression(&input.did)? | ||
| .ok_or_else(|| err("Agent expression not found"))?; | ||
|
|
||
| // Check the key exists in authorised_keys | ||
| let key_exists = current.data.authorised_keys.iter().any(|k| k.key == input.key); | ||
| if !key_exists { | ||
| return Err(err("Key not found in authorised keys")); | ||
| } | ||
|
|
||
| // Check not already revoked | ||
| let already_revoked = current.data.revoked_keys.iter().any(|r| r.revoked_key == input.key); | ||
| if already_revoked { | ||
| return Err(err("Key is already revoked")); | ||
| } | ||
|
|
||
| let now = chrono::Utc::now(); | ||
| let revocation = KeyRevocation { | ||
| revoked_key: input.key.clone(), | ||
| revoked_at: now, | ||
| revoked_by: input.did.clone(), | ||
| signature: input.signature, | ||
| reason: input.reason, | ||
| }; | ||
|
|
||
| let mut new_data = current.data.clone(); | ||
| new_data.authorised_keys.retain(|k| k.key != input.key); | ||
| new_data.revoked_keys.push(revocation); | ||
|
|
||
| let new_expression = AgentExpression { | ||
| author: current.author.clone(), | ||
| timestamp: now, | ||
| data: new_data, | ||
| proof: current.proof.clone(), | ||
| }; | ||
|
|
||
| let did = EntryTypes::Did(Did(current.author.clone())); | ||
| let did_hash = hash_entry(&did)?; | ||
| let entry = EntryTypes::AgentExpression(new_expression.clone()); | ||
| let entry_hash = hash_entry(&entry)?; | ||
| create_entry(&entry)?; | ||
| create_link( | ||
| did_hash, | ||
| entry_hash, | ||
| LinkTypes::ProfileLink, | ||
| LinkTag::new("profile"), | ||
| )?; | ||
|
|
||
| Ok(new_expression) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how get_latest_link works and related utility functions
echo "=== Checking utils.rs for get_latest_link ==="
if [ -f bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/utils.rs ]; then
cat -n bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/utils.rs
else
echo "utils.rs not found, searching for get_latest_link"
rg "get_latest_link" --type rust -B 5 -A 15
fiRepository: coasys/ad4m
Length of output: 1324
🏁 Script executed:
#!/bin/bash
# Check the AgentExpressionData structure for version tracking
echo "=== Checking for AgentExpressionData definition ==="
rg "struct AgentExpressionData" --type rust -A 20Repository: coasys/ad4m
Length of output: 1393
🏁 Script executed:
#!/bin/bash
# Search for version or sequence fields in agent store
echo "=== Checking for version/sequence tracking ==="
rg "version|sequence" bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/ --type rust -B 2 -A 2Repository: coasys/ad4m
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Check how get_current_expression is implemented
echo "=== Checking get_current_expression implementation ==="
rg "fn get_current_expression" --type rust -A 30Repository: coasys/ad4m
Length of output: 3309
🏁 Script executed:
#!/bin/bash
# Search for get_agent_expression implementation
echo "=== Checking get_agent_expression implementation ==="
rg "fn get_agent_expression" --type rust -A 25Repository: coasys/ad4m
Length of output: 2974
🏁 Script executed:
#!/bin/bash
# Check timestamp precision and how it's used
echo "=== Checking timestamp usage and precision ==="
rg "chrono::Utc::now" --type rust -B 2 -A 5Repository: coasys/ad4m
Length of output: 24296
🏁 Script executed:
#!/bin/bash
# Check the full lib.rs to understand the flow better
echo "=== Checking lib.rs structure ==="
wc -l bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs
head -70 bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rsRepository: coasys/ad4m
Length of output: 2296
🏁 Script executed:
#!/bin/bash
# Check how AgentExpression is defined
echo "=== Checking AgentExpression definition ==="
rg "struct AgentExpression" --type rust -A 10Repository: coasys/ad4m
Length of output: 2561
🏁 Script executed:
#!/bin/bash
# Check how add_authorised_key and revoke_key are called
echo "=== Searching for callers of add_authorised_key and revoke_key ==="
rg "add_authorised_key|revoke_key" --type rust -B 2 -A 2 | head -100Repository: coasys/ad4m
Length of output: 8623
🏁 Script executed:
#!/bin/bash
# Check if there are any tests showing concurrent usage
echo "=== Searching for tests in agent_store ==="
fd "test" bootstrap-languages/agent-language/hc-dna/zomes/agent_store/ --type fRepository: coasys/ad4m
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Check for any mutation/locking patterns in the zome
echo "=== Checking full lib.rs for synchronization patterns ==="
cat bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs | tail -100Repository: coasys/ad4m
Length of output: 4371
Read-modify-write race condition in key management functions causes silent data loss.
Both add_authorised_key and revoke_key read the current expression, modify it, and write back a new link—without version tracking or conflict detection. When two operations execute concurrently on the same DID, get_latest_link returns only the link with the latest timestamp, silently discarding whichever mutation was created first. Consider adding a version counter to AgentExpressionData to detect and reject stale writes, or re-read after entry creation to validate the write succeeded as the current state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`
around lines 70 - 188, Both add_authorised_key and revoke_key perform a
read-modify-write on the same DID without concurrency checks, causing lost
updates; add optimistic versioning or post-write validation: extend
AgentExpressionData with a version/seq field (e.g.,
AgentExpressionData::version), have get_current_expression return the version,
on mutation check that current.version matches the version we read, increment
the version on the new AgentExpression before creating the
EntryTypes::AgentExpression, and return an error (or retry) if the version has
changed; alternatively (or additionally) after create_entry of the new
EntryTypes::AgentExpression re-read get_current_expression and verify the latest
entry hash/timestamp/version matches the one you just wrote and abort/rollback
if it does not (apply these checks in both add_authorised_key and revoke_key,
referencing get_current_expression, AgentExpressionData::version, create_entry,
and EntryTypes::AgentExpression).
- Add signature verification in add_authorised_key and revoke_key zome functions - Use Holochain's built-in verify_signature_raw for Ed25519 verification - Decode multibase/multicodec Ed25519 keys (did:key format with z-prefix base58btc) - Decode hex-encoded Ed25519 signatures (64 bytes) Signature message format: UTF-8 bytes of '<subject_key><did><timestamp>' where subject_key is the key being added/revoked (multibase string), did is the full DID string, and timestamp is ISO 8601. Schema changes: - KeyAuthorisation: add 'timestamp' field (signed timestamp) - RevokeKeyInput: add 'revoked_by_key' and 'timestamp' fields - KeyRevocation: add 'revoked_by_key' field Self-signed root keys (signature='self') skip verification. Tests updated to use real Ed25519 keypairs (ed25519-dalek) with valid and invalid signature test cases. Closes coasys#660
- Add serde rename_all camelCase to multi-key structs in integrity zome - Add serde rename for authorisedKeys/revokedKeys on AgentExpressionData - Propagate authorisedKeys/revokedKeys in AgentClient me(), updatePublicPerspective(), updateDirectMessageLanguage() - Refactor add_authorised_key/revoke_key to return AgentExpressionData (adapter signs & stores) - Replace hardcoded resolver stubs with throw Error (requires executor wiring) - Wire KeyAuthorisationInput into agentAddAuthorisedKey mutation - Move Agent JSDoc to correct location above Agent class - Change reason || null to reason ?? null in adapter - Add test for is_key_valid after revocation - Change extract_key_from_did to return Result instead of Option
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs (2)
112-115: Consider adding delimiters to the signature message to prevent theoretical prefix-extension collisions.The concatenated format
subject_key || did || timestampwith no separators is technically ambiguous (though practically safe given the fixeddid:key:and ISO-8601 prefixes). Using a canonical separator (e.g.,\0or|) is a small hardening step.♻️ Proposed change
- let message = format!("{}{}{}", subject_key, did, timestamp); + let message = format!("{}|{}|{}", subject_key, did, timestamp);Update the corresponding client-side signing logic in the adapter/tests to match.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs` around lines 112 - 115, The current signature message is built by concatenating subject_key, did, and timestamp without separators, which can create ambiguous prefix-extension cases; modify the construction where message is created (the variable named message used as input to verify_signature_raw) to insert a canonical delimiter (e.g., "|" or "\0") between subject_key, did, and timestamp, and update the corresponding client-side signing logic in the adapter/tests so the signer and verifier use the exact same delimited format; ensure verify_signature_raw still receives message.into_bytes() after the change.
29-35:unwrap_or(key_str)silently accepts non-multibase-prefixed input.If
key_strlacks the'z'base58btc multibase prefix,strip_prefixreturnsNoneand the raw string is forwarded tobs58::decode. This swallows potential encoding errors. Consider returning an explicitErrfor keys without the expected prefix.♻️ Proposed change
- let without_prefix = key_str.strip_prefix('z').unwrap_or(key_str); - let decoded = bs58::decode(without_prefix) + let without_prefix = key_str + .strip_prefix('z') + .ok_or_else(|| err(&format!("Key missing 'z' multibase prefix: {}", key_str)))?; + let decoded = bs58::decode(without_prefix)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs` around lines 29 - 35, The function decode_ed25519_pubkey currently accepts keys missing the 'z' multibase prefix because it uses strip_prefix('z').unwrap_or(key_str), which lets non-prefixed input proceed to bs58::decode and hides malformed-key errors; change decode_ed25519_pubkey to explicitly validate that key_str starts with 'z' (or return an ExternResult Err with a clear message if it doesn't) before removing the prefix and proceeding to bs58::decode, referencing the parameters key_str and the local variable without_prefix so reviewers can find and update the prefix check and error return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bootstrap-languages/agent-language/adapter.ts`:
- Around line 27-46: The proof parameter of addAuthorisedKey currently declares
authorising_key (snake_case) but the Rust integrity zome (KeyAuthorisation with
#[serde(rename_all = "camelCase")]) expects authorisingKey; update the
TypeScript signature and any usage so the proof object uses authorisingKey
(camelCase) instead of authorising_key, and ensure the object passed into
this.#DNA.call("add_authorised_key", { did, key, name, proof }) contains
proof.authorisingKey; keep createSignedExpression usage the same but ensure the
signedExpression stores the camelCase field so Rust deserialization succeeds.
In
`@bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs`:
- Around line 59-66: The Rust types KeyAuthorisation (field timestamp) and
KeyRevocation (field revoked_by_key serialized as revokedByKey) are exposing
fields that the TypeScript models and GraphQL fragments drop; update the
TypeScript KeyAuthorisation class in core/src/agent/Agent.ts to add timestamp:
string, update the TypeScript KeyRevocation class to add revokedByKey: string
(matching Rust serialization names via camelCase), and extend the AGENT_SUBITEMS
GraphQL fragment in AgentClient.ts to request timestamp and revokedByKey so
these values are returned from the API and preserved for signature/revocation
checks.
- Around line 78-88: The backend KeyRevocation struct includes revoked_by_key
(serialized as revokedByKey) but the TypeScript representation and GraphQL
fragment omitted it; update the TypeScript KeyRevocation class (in Agent.ts) to
add a revokedByKey: string (or string | null if optional) property and adjust
any constructor/deserialization to assign revokedByKey, and update the
AGENT_SUBITEMS revokedKeys GraphQL fragment (in AgentClient.ts) to request
revokedByKey so the field is returned from queries; ensure naming matches the
serialized camelCase key (revokedByKey) used by the backend.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`:
- Around line 203-207: In add_authorised_key, the current guard only checks
current.data.authorised_keys so a key that was revoked by revoke_key (moved into
current.data.revoked_keys) can be re-added; update the validation in
add_authorised_key to also check current.data.revoked_keys (e.g., iterate
revoked_keys and compare k.key == input.key) and return an Err (e.g., "Key has
been revoked" or similar) if found, ensuring revoked keys cannot be
re-authorised.
- Around line 96-116: verify_key_signature currently treats the string "self" as
a sentinel that short-circuits signature verification, which allows
client-controlled inputs in add_authorised_key and revoke_key to bypass Ed25519
checks; modify verify_key_signature to accept an allow_self_signed: bool
parameter (default false) and only return Ok(true) for signature_hex == "self"
when allow_self_signed is true, then update callers: pass allow_self_signed =
false from add_authorised_key and revoke_key (or explicitly check and reject
"self" there before calling verify_key_signature) and pass true only from the
bootstrap path (create_agent_expression) if absolutely needed so normal mutable
operations cannot be spoofed by the "self" sentinel.
In `@core/src/agent/Agent.ts`:
- Around line 59-83: The KeyRevocation GraphQL class is missing the revokedByKey
property present in the Rust struct; add a new non-nullable field property named
revokedByKey (with `@Field`() decorator) to the KeyRevocation class, add it to the
constructor signature, and assign it in the constructor alongside revokedKey,
revokedAt, revokedBy, signature, and reason so instances include the authorising
key that signed the revocation.
- Around line 5-31: Add a timestamp:string field to both GraphQL types so the
signed message can round-trip: update the KeyAuthorisation class and
KeyAuthorisationInput class to declare a `@Field`() timestamp: string, accept
timestamp in each constructor signature, and assign this.timestamp = timestamp
in their constructors; keep the existing authorisingKey and signature members
and decorators intact so signature verification using the
"<subject_key><did><timestamp>" value will be possible.
In `@core/src/agent/AgentClient.ts`:
- Around line 35-51: The AGENT_SUBITEMS GraphQL fragment in AgentClient.ts is
missing proof.timestamp and revokedKeys.revokedByKey; update the AGENT_SUBITEMS
fragment to request proof { timestamp authorisingKey signature } and revokedKeys
{ revokedByKey revokedKey revokedAt revokedBy signature reason } and also add
the corresponding fields to the GraphQL schema/types (the Proof and RevokedKey
types) so the generated client types include these properties, then regenerate
GraphQL types/clients.
---
Duplicate comments:
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`:
- Around line 221-222: The current flow returns AgentExpressionData without
persisting it, widening TOCTOU races (get_latest_link) and allowing direct-write
bypass because create_agent_expression is a public #[hdk_extern] with no auth
checks; fix by moving validation and the write into the same zome function
(merge the logic from add_authorised_key/revoke_key into the persistence path)
so the function that validates authorised_keys/revoked_keys also calls
create_entry atomically, or alternatively implement integrity-zome validation
callbacks that enforce the same rules on AgentExpression entries and make
create_agent_expression non-public or add explicit authorization checks to it;
update references to create_agent_expression, add_authorised_key, revoke_key,
get_latest_link, and AgentExpression(AgentExpressionData) accordingly.
In `@core/src/agent/AgentResolver.ts`:
- Around line 268-306: agentRevokeKey currently only takes key and reason but
the DNA's RevokeKeyInput requires revoked_by_key, signature and timestamp —
update the resolver method signature agentRevokeKey to accept those additional
args (revoked_by_key: string, signature: string, timestamp: number or string as
per DNA) and pass them through to the executor/zome when wiring; also ensure the
GraphQL input type KeyAuthorisationInput (used by agentAddAuthorisedKey)
includes a timestamp field so signature-verified operations have the required
timestamp metadata.
---
Nitpick comments:
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`:
- Around line 112-115: The current signature message is built by concatenating
subject_key, did, and timestamp without separators, which can create ambiguous
prefix-extension cases; modify the construction where message is created (the
variable named message used as input to verify_signature_raw) to insert a
canonical delimiter (e.g., "|" or "\0") between subject_key, did, and timestamp,
and update the corresponding client-side signing logic in the adapter/tests so
the signer and verifier use the exact same delimited format; ensure
verify_signature_raw still receives message.into_bytes() after the change.
- Around line 29-35: The function decode_ed25519_pubkey currently accepts keys
missing the 'z' multibase prefix because it uses
strip_prefix('z').unwrap_or(key_str), which lets non-prefixed input proceed to
bs58::decode and hides malformed-key errors; change decode_ed25519_pubkey to
explicitly validate that key_str starts with 'z' (or return an ExternResult Err
with a clear message if it doesn't) before removing the prefix and proceeding to
bs58::decode, referencing the parameters key_str and the local variable
without_prefix so reviewers can find and update the prefix check and error
return.
Comment on lines
+27
to
+46
| async addAuthorisedKey(did: string, key: string, name: string, proof: { authorising_key: string, signature: string, timestamp: string }): Promise<any> { | ||
| // Zome validates and returns updated AgentExpressionData | ||
| const updatedData = await this.#DNA.call( | ||
| DNA_ROLE, | ||
| ZOME_NAME, | ||
| "add_authorised_key", | ||
| { did, key, name, proof } | ||
| ); | ||
|
|
||
| // Re-sign with the agent's key and store the full expression | ||
| const signedExpression = this.#agent.createSignedExpression(updatedData); | ||
| await this.#DNA.call( | ||
| DNA_ROLE, | ||
| ZOME_NAME, | ||
| "create_agent_expression", | ||
| signedExpression | ||
| ); | ||
|
|
||
| return signedExpression; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for KeyAuthorisation and AddAuthorisedKeyInput structs in Rust files
echo "=== Searching for KeyAuthorisation struct ==="
fd -t f "\.rs$" bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/ | xargs rg -l "KeyAuthorisation" | head -5
echo ""
echo "=== Searching for serde rename_all in agent_store ==="
fd -t f "\.rs$" bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/ | xargs rg -B3 "KeyAuthorisation|AddAuthorisedKeyInput" | head -80Repository: coasys/ad4m
Length of output: 720
🏁 Script executed:
#!/bin/bash
# Check the exact TS type signature in adapter.ts
echo "=== Checking proof parameter in addAuthorisedKey ==="
rg -n "addAuthorisedKey" bootstrap-languages/agent-language/adapter.ts -A5 -B1
echo ""
echo "=== Check revokeKey signature ==="
rg -n "revokeKey" bootstrap-languages/agent-language/adapter.ts -A5 -B1Repository: coasys/ad4m
Length of output: 808
🏁 Script executed:
#!/bin/bash
# Check for any type definitions for proof in TS files
echo "=== Searching for proof type definitions ==="
fd -t f "\.ts$" bootstrap-languages/agent-language/ | xargs rg -B2 -A2 "authorising_key|authorisingKey" 2>/dev/null | head -100Repository: coasys/ad4m
Length of output: 598
🏁 Script executed:
#!/bin/bash
# Get full KeyAuthorisation struct definition
echo "=== Full KeyAuthorisation struct ==="
rg -A15 "pub struct KeyAuthorisation" bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs
echo ""
echo "=== Verify the call serialization method ==="
rg -n "this\.#DNA\.call" bootstrap-languages/agent-language/adapter.ts -B2 -A8 | head -50Repository: coasys/ad4m
Length of output: 2308
🏁 Script executed:
#!/bin/bash
# Check the HolochainLanguageDelegate or DNA call implementation
echo "=== Search for DNA.call or HolochainLanguageDelegate implementation ==="
fd -t f "\.ts$" bootstrap-languages/agent-language/ | xargs rg -l "DNA.*call|HolochainLanguageDelegate" | head -10
echo ""
echo "=== Check imports and DNA definition in adapter.ts ==="
rg -n "import|#DNA|DNA_ROLE" bootstrap-languages/agent-language/adapter.ts | head -20Repository: coasys/ad4m
Length of output: 1133
🏁 Script executed:
#!/bin/bash
# Search for any snake_case to camelCase conversion middleware
echo "=== Search for camelCase conversion ==="
fd -t f "\.ts$" bootstrap-languages/agent-language/ | xargs rg "camelCase|snakeCase|transformKeys|keysToCase" 2>/dev/null | head -20Repository: coasys/ad4m
Length of output: 94
Serialization mismatch: proof.authorising_key uses snake_case but Rust expects camelCase.
The proof parameter declares authorising_key (snake_case), but KeyAuthorisation in the integrity zome uses #[serde(rename_all = "camelCase")], expecting the JSON key "authorisingKey". When the object is serialized, { authorising_key: "..." } becomes JSON "authorising_key", causing deserialization to fail.
Update the TS type signature to use camelCase:
🐛 Proposed fix
- async addAuthorisedKey(did: string, key: string, name: string, proof: { authorising_key: string, signature: string, timestamp: string }): Promise<any> {
+ async addAuthorisedKey(did: string, key: string, name: string, proof: { authorisingKey: string, signature: string, timestamp: string }): Promise<any> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async addAuthorisedKey(did: string, key: string, name: string, proof: { authorising_key: string, signature: string, timestamp: string }): Promise<any> { | |
| // Zome validates and returns updated AgentExpressionData | |
| const updatedData = await this.#DNA.call( | |
| DNA_ROLE, | |
| ZOME_NAME, | |
| "add_authorised_key", | |
| { did, key, name, proof } | |
| ); | |
| // Re-sign with the agent's key and store the full expression | |
| const signedExpression = this.#agent.createSignedExpression(updatedData); | |
| await this.#DNA.call( | |
| DNA_ROLE, | |
| ZOME_NAME, | |
| "create_agent_expression", | |
| signedExpression | |
| ); | |
| return signedExpression; | |
| } | |
| async addAuthorisedKey(did: string, key: string, name: string, proof: { authorisingKey: string, signature: string, timestamp: string }): Promise<any> { | |
| // Zome validates and returns updated AgentExpressionData | |
| const updatedData = await this.#DNA.call( | |
| DNA_ROLE, | |
| ZOME_NAME, | |
| "add_authorised_key", | |
| { did, key, name, proof } | |
| ); | |
| // Re-sign with the agent's key and store the full expression | |
| const signedExpression = this.#agent.createSignedExpression(updatedData); | |
| await this.#DNA.call( | |
| DNA_ROLE, | |
| ZOME_NAME, | |
| "create_agent_expression", | |
| signedExpression | |
| ); | |
| return signedExpression; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-languages/agent-language/adapter.ts` around lines 27 - 46, The
proof parameter of addAuthorisedKey currently declares authorising_key
(snake_case) but the Rust integrity zome (KeyAuthorisation with
#[serde(rename_all = "camelCase")]) expects authorisingKey; update the
TypeScript signature and any usage so the proof object uses authorisingKey
(camelCase) instead of authorising_key, and ensure the object passed into
this.#DNA.call("add_authorised_key", { did, key, name, proof }) contains
proof.authorisingKey; keep createSignedExpression usage the same but ensure the
signedExpression stores the camelCase field so Rust deserialization succeeds.
Comment on lines
+59
to
+66
| #[derive(Serialize, Deserialize, Clone, SerializedBytes, Debug)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct KeyAuthorisation { | ||
| pub authorising_key: String, | ||
| pub signature: String, | ||
| /// ISO 8601 timestamp that was included in the signed message | ||
| pub timestamp: String, | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "Agent.ts" -type f | head -5Repository: coasys/ad4m
Length of output: 80
🏁 Script executed:
find . -name "AgentClient.ts" -type f | head -5Repository: coasys/ad4m
Length of output: 86
🏁 Script executed:
rg -n "class KeyAuthorisation" -A 20 --type=tsRepository: coasys/ad4m
Length of output: 1597
🏁 Script executed:
rg -n "authorisingKey" --type=ts -B 5 -A 5Repository: coasys/ad4m
Length of output: 2668
🏁 Script executed:
rg -n "class KeyRevocation" -A 20 --type=tsRepository: coasys/ad4m
Length of output: 1072
🏁 Script executed:
rg -n "revokedKey" --type=ts -B 2 -A 2 | head -40Repository: coasys/ad4m
Length of output: 2176
🏁 Script executed:
cat -n bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs | sed -n '78,88p'Repository: coasys/ad4m
Length of output: 498
🏁 Script executed:
cat -n bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs | sed -n '59,66p'Repository: coasys/ad4m
Length of output: 396
Add timestamp field to TypeScript KeyAuthorisation type and GraphQL queries.
The Rust KeyAuthorisation struct includes a timestamp: String field (lines 61-65, serialized as camelCase), documenting the ISO 8601 timestamp that was included in the signed message. The corresponding TypeScript KeyAuthorisation class in core/src/agent/Agent.ts (lines 6-17) and the AGENT_SUBITEMS GraphQL fragment in AgentClient.ts (lines 40-43) only define authorisingKey and signature. The timestamp is silently dropped during GraphQL queries, losing data critical for signature verification context.
Additionally, the Rust KeyRevocation struct includes a revoked_by_key: String field (line 85, serialized as revokedByKey), but this field is missing from the TypeScript KeyRevocation class and GraphQL queries, also causing silent data loss.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs`
around lines 59 - 66, The Rust types KeyAuthorisation (field timestamp) and
KeyRevocation (field revoked_by_key serialized as revokedByKey) are exposing
fields that the TypeScript models and GraphQL fragments drop; update the
TypeScript KeyAuthorisation class in core/src/agent/Agent.ts to add timestamp:
string, update the TypeScript KeyRevocation class to add revokedByKey: string
(matching Rust serialization names via camelCase), and extend the AGENT_SUBITEMS
GraphQL fragment in AgentClient.ts to request timestamp and revokedByKey so
these values are returned from the API and preserved for signature/revocation
checks.
Comment on lines
+78
to
+88
| #[derive(Serialize, Deserialize, Clone, SerializedBytes, Debug)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct KeyRevocation { | ||
| pub revoked_key: String, | ||
| pub revoked_at: DateTime<Utc>, | ||
| pub revoked_by: String, | ||
| /// The authorised key that signed this revocation | ||
| pub revoked_by_key: String, | ||
| pub signature: String, | ||
| pub reason: Option<String>, | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the TypeScript KeyRevocation class
echo "=== Looking for TypeScript KeyRevocation class ==="
find . -name "Agent.ts" -type f | head -5Repository: coasys/ad4m
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Search for KeyRevocation in TypeScript files
echo "=== Searching for KeyRevocation in TypeScript ==="
rg -n "class KeyRevocation|interface KeyRevocation|type KeyRevocation" --type=ts --type=tsxRepository: coasys/ad4m
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Search for revokedByKey or revoked_by_key in TypeScript
echo "=== Searching for revokedByKey field ==="
rg -n "revokedByKey" --type=ts --type=tsxRepository: coasys/ad4m
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Search for GraphQL query fragments with revokedKeys
echo "=== Searching for revokedKeys in GraphQL/TS files ==="
rg -n "revokedKey" --type=ts --type=tsx -B3 -A3Repository: coasys/ad4m
Length of output: 153
🏁 Script executed:
#!/bin/bash
# Verify the Rust struct has revoked_by_key
echo "=== Verifying Rust KeyRevocation struct ==="
find . -name "lib.rs" -path "*/agent_store_integrity/*" -type f | head -3Repository: coasys/ad4m
Length of output: 181
🏁 Script executed:
#!/bin/bash
# Search for the exact file path mentioned in the review
echo "=== Looking for the agent_store_integrity lib.rs ==="
fd "lib.rs" -p "*agent_store_integrity*"Repository: coasys/ad4m
Length of output: 235
🏁 Script executed:
#!/bin/bash
# Read the Agent.ts file to see KeyRevocation class
echo "=== Agent.ts KeyRevocation class ==="
sed -n '59,100p' ./core/src/agent/Agent.tsRepository: coasys/ad4m
Length of output: 1439
🏁 Script executed:
#!/bin/bash
# Search for revokedByKey without tsx type
echo "=== Searching for revokedByKey ==="
rg -n "revokedByKey" --type=tsRepository: coasys/ad4m
Length of output: 403
🏁 Script executed:
#!/bin/bash
# Search for revokedKey patterns
echo "=== Searching for revokedKey ==="
rg -n "revokedKey" --type=ts -B2 -A2Repository: coasys/ad4m
Length of output: 3032
🏁 Script executed:
#!/bin/bash
# Read the Rust lib.rs to verify revoked_by_key field
echo "=== Rust KeyRevocation struct ==="
sed -n '78,88p' ./bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rsRepository: coasys/ad4m
Length of output: 455
🏁 Script executed:
#!/bin/bash
# Search for AgentClient.ts and GraphQL fragments
echo "=== Looking for AgentClient.ts ==="
find . -name "AgentClient.ts" -o -name "AgentClient.js" | head -5Repository: coasys/ad4m
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Search for AGENT_SUBITEMS in TypeScript files
echo "=== Searching for AGENT_SUBITEMS ==="
rg -n "AGENT_SUBITEMS" --type=ts -A10Repository: coasys/ad4m
Length of output: 5664
🏁 Script executed:
#!/bin/bash
# Get full AGENT_SUBITEMS definition
echo "=== Full AGENT_SUBITEMS fragment ==="
sed -n '21,50p' ./core/src/agent/AgentClient.tsRepository: coasys/ad4m
Length of output: 654
revokedByKey field missing from TypeScript KeyRevocation type and GraphQL queries.
The Rust KeyRevocation struct includes revoked_by_key (serialized as revokedByKey), but the TypeScript KeyRevocation class in core/src/agent/Agent.ts (lines 60-75) and the AGENT_SUBITEMS revokedKeys fragment in AgentClient.ts (lines 45-48) do not include this field. The backend sends this data, but the GraphQL query does not request it and the TypeScript type cannot store it, causing silent data loss.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@bootstrap-languages/agent-language/hc-dna/zomes/agent_store_integrity/src/lib.rs`
around lines 78 - 88, The backend KeyRevocation struct includes revoked_by_key
(serialized as revokedByKey) but the TypeScript representation and GraphQL
fragment omitted it; update the TypeScript KeyRevocation class (in Agent.ts) to
add a revokedByKey: string (or string | null if optional) property and adjust
any constructor/deserialization to assign revokedByKey, and update the
AGENT_SUBITEMS revokedKeys GraphQL fragment (in AgentClient.ts) to request
revokedByKey so the field is returned from queries; ensure naming matches the
serialized camelCase key (revokedByKey) used by the backend.
Comment on lines
+96
to
+116
| fn verify_key_signature( | ||
| signing_key_str: &str, | ||
| signature_hex: &str, | ||
| subject_key: &str, | ||
| did: &str, | ||
| timestamp: &str, | ||
| ) -> ExternResult<bool> { | ||
| // Skip verification for self-signed root keys | ||
| if signature_hex == "self" { | ||
| return Ok(true); | ||
| } | ||
|
|
||
| let pubkey_bytes = decode_ed25519_pubkey(signing_key_str)?; | ||
| let agent_pubkey = AgentPubKey::from_raw_32(pubkey_bytes.to_vec()); | ||
| let signature = decode_signature(signature_hex)?; | ||
|
|
||
| // Message = subject_key + did + timestamp (UTF-8 bytes) | ||
| let message = format!("{}{}{}", subject_key, did, timestamp); | ||
|
|
||
| verify_signature_raw(agent_pubkey, signature, message.into_bytes()) | ||
| } |
Contributor
There was a problem hiding this comment.
"self" sentinel in verify_key_signature creates a signature-bypass for add_authorised_key and revoke_key.
verify_key_signature is called both during bootstrap (where "self" is a legitimate sentinel for the root key in create_agent_expression) and from add_authorised_key / revoke_key where signature_hex is a client-controlled field (input.proof.signature, input.signature). Any caller can pass "self" in those fields to skip the Ed25519 check entirely—needing only knowledge of any authorized key's public string (which is visible on the DHT) to satisfy the preceding list-membership checks.
The fix is to not accept "self" inside the general verifier used for mutable operations, or at minimum to reject it in add_authorised_key / revoke_key before calling verify_key_signature.
🔒 Proposed fix
fn verify_key_signature(
signing_key_str: &str,
signature_hex: &str,
subject_key: &str,
did: &str,
timestamp: &str,
+ allow_self_signed: bool,
) -> ExternResult<bool> {
- // Skip verification for self-signed root keys
- if signature_hex == "self" {
+ if signature_hex == "self" && allow_self_signed {
return Ok(true);
}
+ if signature_hex == "self" {
+ return Err(err("'self' sentinel is not valid here"));
+ }Pass allow_self_signed: false from add_authorised_key and revoke_key, and true only from the bootstrap path (if needed at all — create_agent_expression stores the proof as-is without calling verify_key_signature).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`
around lines 96 - 116, verify_key_signature currently treats the string "self"
as a sentinel that short-circuits signature verification, which allows
client-controlled inputs in add_authorised_key and revoke_key to bypass Ed25519
checks; modify verify_key_signature to accept an allow_self_signed: bool
parameter (default false) and only return Ok(true) for signature_hex == "self"
when allow_self_signed is true, then update callers: pass allow_self_signed =
false from add_authorised_key and revoke_key (or explicitly check and reject
"self" there before calling verify_key_signature) and pass true only from the
bootstrap path (create_agent_expression) if absolutely needed so normal mutable
operations cannot be spoofed by the "self" sentinel.
Comment on lines
+203
to
+207
| // Check the new key isn't already authorised | ||
| let already_exists = current.data.authorised_keys.iter().any(|k| k.key == input.key); | ||
| if already_exists { | ||
| return Err(err("Key is already authorised")); | ||
| } |
Contributor
There was a problem hiding this comment.
add_authorised_key allows re-authorization of a previously revoked key.
The already_exists guard only checks authorised_keys. After revoke_key moves a key to revoked_keys and removes it from authorised_keys, already_exists returns false for that key, so it can be re-added with a fresh valid signature—circumventing revocation.
🛡️ Proposed fix
// Check the new key isn't already authorised
let already_exists = current.data.authorised_keys.iter().any(|k| k.key == input.key);
if already_exists {
return Err(err("Key is already authorised"));
}
+
+ // Check the new key was not previously revoked
+ let was_revoked = current.data.revoked_keys.iter().any(|r| r.revoked_key == input.key);
+ if was_revoked {
+ return Err(err("Cannot re-authorize a revoked key"));
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bootstrap-languages/agent-language/hc-dna/zomes/agent_store/src/lib.rs`
around lines 203 - 207, In add_authorised_key, the current guard only checks
current.data.authorised_keys so a key that was revoked by revoke_key (moved into
current.data.revoked_keys) can be re-added; update the validation in
add_authorised_key to also check current.data.revoked_keys (e.g., iterate
revoked_keys and compare k.key == input.key) and return an Err (e.g., "Key has
been revoked" or similar) if found, ensuring revoked keys cannot be
re-authorised.
Comment on lines
+5
to
+31
| @ObjectType() | ||
| export class KeyAuthorisation { | ||
| @Field() | ||
| authorisingKey: string; | ||
|
|
||
| @Field() | ||
| signature: string; | ||
|
|
||
| constructor(authorisingKey: string, signature: string) { | ||
| this.authorisingKey = authorisingKey; | ||
| this.signature = signature; | ||
| } | ||
| } | ||
|
|
||
| @InputType() | ||
| export class KeyAuthorisationInput { | ||
| @Field() | ||
| authorisingKey: string; | ||
|
|
||
| @Field() | ||
| signature: string; | ||
|
|
||
| constructor(authorisingKey: string, signature: string) { | ||
| this.authorisingKey = authorisingKey; | ||
| this.signature = signature; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
KeyAuthorisation and KeyAuthorisationInput are missing the timestamp field required for signature verification.
The Rust KeyAuthorisation struct includes a timestamp: String field that holds the ISO 8601 timestamp included in the signed message (<subject_key><did><timestamp>). Without this field in the GraphQL types, the timestamp cannot round-trip through the API, and signature verification will be impossible when the resolver is wired up.
🔧 Proposed fix
`@ObjectType`()
export class KeyAuthorisation {
`@Field`()
authorisingKey: string;
`@Field`()
signature: string;
- constructor(authorisingKey: string, signature: string) {
+ `@Field`()
+ timestamp: string;
+
+ constructor(authorisingKey: string, signature: string, timestamp: string) {
this.authorisingKey = authorisingKey;
this.signature = signature;
+ this.timestamp = timestamp;
}
}
`@InputType`()
export class KeyAuthorisationInput {
`@Field`()
authorisingKey: string;
`@Field`()
signature: string;
- constructor(authorisingKey: string, signature: string) {
+ `@Field`()
+ timestamp: string;
+
+ constructor(authorisingKey: string, signature: string, timestamp: string) {
this.authorisingKey = authorisingKey;
this.signature = signature;
+ this.timestamp = timestamp;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ObjectType() | |
| export class KeyAuthorisation { | |
| @Field() | |
| authorisingKey: string; | |
| @Field() | |
| signature: string; | |
| constructor(authorisingKey: string, signature: string) { | |
| this.authorisingKey = authorisingKey; | |
| this.signature = signature; | |
| } | |
| } | |
| @InputType() | |
| export class KeyAuthorisationInput { | |
| @Field() | |
| authorisingKey: string; | |
| @Field() | |
| signature: string; | |
| constructor(authorisingKey: string, signature: string) { | |
| this.authorisingKey = authorisingKey; | |
| this.signature = signature; | |
| } | |
| } | |
| `@ObjectType`() | |
| export class KeyAuthorisation { | |
| `@Field`() | |
| authorisingKey: string; | |
| `@Field`() | |
| signature: string; | |
| `@Field`() | |
| timestamp: string; | |
| constructor(authorisingKey: string, signature: string, timestamp: string) { | |
| this.authorisingKey = authorisingKey; | |
| this.signature = signature; | |
| this.timestamp = timestamp; | |
| } | |
| } | |
| `@InputType`() | |
| export class KeyAuthorisationInput { | |
| `@Field`() | |
| authorisingKey: string; | |
| `@Field`() | |
| signature: string; | |
| `@Field`() | |
| timestamp: string; | |
| constructor(authorisingKey: string, signature: string, timestamp: string) { | |
| this.authorisingKey = authorisingKey; | |
| this.signature = signature; | |
| this.timestamp = timestamp; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/agent/Agent.ts` around lines 5 - 31, Add a timestamp:string field to
both GraphQL types so the signed message can round-trip: update the
KeyAuthorisation class and KeyAuthorisationInput class to declare a `@Field`()
timestamp: string, accept timestamp in each constructor signature, and assign
this.timestamp = timestamp in their constructors; keep the existing
authorisingKey and signature members and decorators intact so signature
verification using the "<subject_key><did><timestamp>" value will be possible.
Comment on lines
+59
to
+83
| @ObjectType() | ||
| export class KeyRevocation { | ||
| @Field() | ||
| revokedKey: string; | ||
|
|
||
| @Field() | ||
| revokedAt: string; | ||
|
|
||
| @Field() | ||
| revokedBy: string; | ||
|
|
||
| @Field() | ||
| signature: string; | ||
|
|
||
| @Field({ nullable: true }) | ||
| reason?: string; | ||
|
|
||
| constructor(revokedKey: string, revokedAt: string, revokedBy: string, signature: string, reason?: string) { | ||
| this.revokedKey = revokedKey; | ||
| this.revokedAt = revokedAt; | ||
| this.revokedBy = revokedBy; | ||
| this.signature = signature; | ||
| this.reason = reason; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
KeyRevocation is missing the revokedByKey field present in the Rust struct.
The Rust KeyRevocation includes revoked_by_key (serialized as revokedByKey) — the specific authorised key that signed the revocation. Without this field, clients can't determine which key authorized a revocation.
🔧 Proposed fix
`@ObjectType`()
export class KeyRevocation {
`@Field`()
revokedKey: string;
`@Field`()
revokedAt: string;
`@Field`()
revokedBy: string;
+ `@Field`()
+ revokedByKey: string;
+
`@Field`()
signature: string;
`@Field`({ nullable: true })
reason?: string;
- constructor(revokedKey: string, revokedAt: string, revokedBy: string, signature: string, reason?: string) {
+ constructor(revokedKey: string, revokedAt: string, revokedBy: string, revokedByKey: string, signature: string, reason?: string) {
this.revokedKey = revokedKey;
this.revokedAt = revokedAt;
this.revokedBy = revokedBy;
+ this.revokedByKey = revokedByKey;
this.signature = signature;
this.reason = reason;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/agent/Agent.ts` around lines 59 - 83, The KeyRevocation GraphQL
class is missing the revokedByKey property present in the Rust struct; add a new
non-nullable field property named revokedByKey (with `@Field`() decorator) to the
KeyRevocation class, add it to the constructor signature, and assign it in the
constructor alongside revokedKey, revokedAt, revokedBy, signature, and reason so
instances include the authorising key that signed the revocation.
Comment on lines
+35
to
+51
| authorisedKeys { | ||
| key | ||
| name | ||
| addedAt | ||
| addedBy | ||
| proof { | ||
| authorisingKey | ||
| signature | ||
| } | ||
| } | ||
| revokedKeys { | ||
| revokedKey | ||
| revokedAt | ||
| revokedBy | ||
| signature | ||
| reason | ||
| } |
Contributor
There was a problem hiding this comment.
AGENT_SUBITEMS GraphQL fragment missing timestamp in proof and revokedByKey in revokedKeys.
These fields exist in the Rust data model but are absent from both the GraphQL types and this query fragment, so they will not be returned to clients.
🔧 Proposed fix
authorisedKeys {
key
name
addedAt
addedBy
proof {
authorisingKey
signature
+ timestamp
}
}
revokedKeys {
revokedKey
revokedAt
revokedBy
+ revokedByKey
signature
reason
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| authorisedKeys { | |
| key | |
| name | |
| addedAt | |
| addedBy | |
| proof { | |
| authorisingKey | |
| signature | |
| } | |
| } | |
| revokedKeys { | |
| revokedKey | |
| revokedAt | |
| revokedBy | |
| signature | |
| reason | |
| } | |
| authorisedKeys { | |
| key | |
| name | |
| addedAt | |
| addedBy | |
| proof { | |
| authorisingKey | |
| signature | |
| timestamp | |
| } | |
| } | |
| revokedKeys { | |
| revokedKey | |
| revokedAt | |
| revokedBy | |
| revokedByKey | |
| signature | |
| reason | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/agent/AgentClient.ts` around lines 35 - 51, The AGENT_SUBITEMS
GraphQL fragment in AgentClient.ts is missing proof.timestamp and
revokedKeys.revokedByKey; update the AGENT_SUBITEMS fragment to request proof {
timestamp authorisingKey signature } and revokedKeys { revokedByKey revokedKey
revokedAt revokedBy signature reason } and also add the corresponding fields to
the GraphQL schema/types (the Proof and RevokedKey types) so the generated
client types include these properties, then regenerate GraphQL types/clients.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Multi-Key Agent Identity Support
Implements multi-key support for agent identity as outlined in #660.
Phase 1: Data Model + Zome Updates
AuthorisedKey,KeyAuthorisation,KeyRevocationstructsAgentExpressionData: Extended withauthorised_keysandrevoked_keysfields (#[serde(default)]for backward compatibility)add_authorised_key,revoke_key,is_key_validcreate_agent_expressionauto-populates the DID root key as first authorised key whenauthorised_keysis emptyPhase 2: Client API
AuthorisedKey,KeyAuthorisation,KeyRevocationwith@ObjectType()/@InputType()decoratorsagentAddAuthorisedKey(key, name),agentRevokeKey(key, reason?)agentAuthorisedKeys,agentRevokedKeys,agentIsKeyValid(did, key)addAuthorisedKey(),revokeKey(),authorisedKeys(),revokedKeys(),isKeyValid()Test Coverage (NEW)
did:keyDIDsis_key_validcorrectness for authorised, revoked, and unknown keysBackward Compatibility
Vecfields use#[serde(default)]— existing agent expressions deserialize without issuesdid:key:...) is always the implicit root keyThese issues are by-design for Phase 1 (data model only) but must be addressed before production use:
No cryptographic signature verification —
add_authorised_keyandrevoke_keystore signatures but never verify them cryptographically. Theproof.signaturefield is currently just stored, not validated against the authorising key. This means any caller who knows an authorised key string can add arbitrary new keys.No caller authorization — Any Holochain agent can call
add_authorised_keyorrevoke_keyfor any DID. There is no check that the zome caller owns or controls the DID. This needs to be enforced either in the zome (viaagent_info()) or at a higher layer.revoke_keydoesn't verify who is revoking — The revocation storesrevoked_byas the DID but doesn't verify the caller has authority to revoke (i.e., holds an authorised key).Root key uses
"self"as signature — The auto-populated root key hasproof.signature = "self"which is a sentinel value. This is fine for bootstrapping but should be documented as a special case.These are tracked for Phase 3 (signature verification + authorization enforcement).
Related to #660 (Phases 1 & 2 only)
Summary by CodeRabbit
New Features
Tests