fix: upstream AzureClaw SDK patches + add GovernanceService proto (#557, #558)

[imran-siddique]

Upstreams bug fixes from AzureClaw's vendored agentmesh-sdk and adds the GovernanceService proto for language-agnostic integration.

SDK Bug Fixes (#557)

AzureClaw vendors a patched agentmesh-sdk because keys with type prefixes (\�d25519:, \x25519:) fail to decode. These fixes allow AzureClaw to drop the vendor directory and depend on official releases.

Fix	File	Change
Key prefix stripping	identity.ts	\stripKeyPrefix()\ strips \�d25519:/\x25519:\ before base64 decode
Safe base64 decode	identity.ts	\safeBase64Decode()\ wraps prefix-aware decoding
fromJSON round-trip	identity.ts	Uses \safeBase64Decode()\ so prefixed keys deserialize correctly

GovernanceService Proto (#558)

Adds gRPC service definition for language-agnostic governance operations:

\\protobuf
service GovernanceService {
rpc EvaluatePolicy (PolicyRequest) returns (PolicyDecision);
rpc RecordAudit (AuditEntry) returns (AuditAck);
rpc GetTrustScore (TrustQuery) returns (TrustScoreResult);
}
\\

Tests

12 new tests covering prefix stripping, safe decode, and fromJSON round-trip with prefixed keys. All 149 TS SDK tests pass.

Closes #557, Closes #558

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces two major changes:

1. SDK Bug Fixes (#557): Fixes issues with key decoding in the agentmesh-sdk by implementing stripKeyPrefix and safeBase64Decode functions. These changes address issues with keys that include type prefixes (e.g., ed25519: or x25519:).

2. GovernanceService Proto (#558): Adds a new GovernanceService gRPC service definition for policy evaluation, audit recording, and trust score queries. This is a significant addition that enables language-agnostic integration for governance operations.

The changes are well-documented, and the new functionality is accompanied by comprehensive test coverage. However, there are a few areas that require attention, including potential security concerns, API design considerations, and opportunities for improvement.

---

🔴 CRITICAL

1. Key Prefix Handling - Security Implications:

   • The stripKeyPrefix function logs a warning when it encounters an unexpected prefix but still proceeds to strip the prefix and use the remaining key. This could lead to unintended behavior or security vulnerabilities if an attacker intentionally crafts a key with a misleading prefix. For example:

     const result = stripKeyPrefix('malicious_prefix:actual_key', 'ed25519:');

     In this case, the function would strip the malicious_prefix and use actual_key, potentially leading to a security bypass.
   • Recommendation: Instead of stripping unexpected prefixes, the function should throw an error or reject keys with unrecognized prefixes. This ensures that only explicitly allowed prefixes are processed.

2. AuditEntry Hash Validation:

   • The AuditEntry message includes a hash and previous_hash field for tamper-evident logging. However, there is no indication in the PR that these hashes are validated or verified in the implementation. Without proper validation, the tamper-evident log could be compromised.
   • Recommendation: Ensure that the hash and previous_hash fields are cryptographically verified during the RecordAudit RPC. This should be explicitly documented in the proto file and implemented in the corresponding service logic.

3. Trust Score Range Validation:

   • The TrustScoreResult message includes an overall_score field with a range of 0-1000. However, there is no validation or enforcement of this range in the proto definition or the implementation.
   • Recommendation: Add validation logic to ensure that the overall_score falls within the specified range. This can be done using custom validation logic in the service implementation.

---

🟡 WARNING

1. Breaking Changes in TypeScript SDK:

   • The AgentIdentity.fromJSON method now uses the new safeBase64Decode function, which changes the behavior for keys with prefixes. While this is a bug fix, it could potentially break existing integrations if users were relying on the old behavior.
   • Recommendation: Clearly document this change in the release notes and consider bumping the major version of the SDK to indicate a breaking change.

2. GovernanceService Proto - Backward Compatibility:

   • Adding a new gRPC service (GovernanceService) is a backward-compatible change. However, any future changes to this service (e.g., adding new fields to messages or modifying RPC methods) could introduce breaking changes.
   • Recommendation: Follow best practices for proto evolution, such as using reserved field numbers for removed fields and avoiding changes to existing field numbers or types.

---

💡 SUGGESTIONS

1. Improved Error Handling in stripKeyPrefix:

   • Instead of logging a warning for unexpected prefixes, consider returning an error or throwing an exception. This would make the function's behavior more predictable and reduce the risk of security issues.
   • Example:

     if (!keyStr.startsWith(expectedPrefix)) {
       throw new Error(`Unexpected key prefix. Expected '${expectedPrefix}'`);
     }

2. Test Coverage for Edge Cases:

   • While the test suite is comprehensive, consider adding tests for the following edge cases:
     • Keys with multiple colons (e.g., ed25519:x25519:AAAA).
     • Extremely long or malformed keys.
     • Empty or null context maps in PolicyRequest.

3. Documentation for GovernanceService:

   • The GovernanceService proto is well-documented, but additional documentation should be provided for developers integrating with this service. For example:
     • Examples of valid PolicyRequest and AuditEntry messages.
     • Explanation of how TrustScoreDimensions is calculated and used.

4. Thread Safety in GovernanceService:

   • If the GovernanceService implementation involves shared state (e.g., for the audit log or trust score calculations), ensure that proper synchronization mechanisms are in place to avoid race conditions.
   • Recommendation: Use thread-safe data structures or synchronization primitives (e.g., locks) to manage shared state.

5. Consider Using a More Secure Base64 Decoder:

   • The safeBase64Decode function uses Node.js's Buffer.from method for base64 decoding. While this is generally safe, consider using a library that explicitly handles edge cases like padding or invalid characters (e.g., base64url).

6. Policy Evaluation Context Validation:

   • The PolicyRequest message includes a context map for additional metadata. Ensure that the keys and values in this map are validated to prevent injection attacks or unexpected behavior in the policy engine.

7. Logging Sensitive Information:

   • The console.warn statements in stripKeyPrefix may inadvertently log sensitive information (e.g., key material). Avoid logging sensitive data or ensure that logs are sanitized.

---

Final Assessment

• The PR addresses critical bugs and introduces valuable new functionality with the GovernanceService proto.
• However, there are critical security concerns related to key prefix handling and audit log validation that must be addressed before merging.
• Additionally, the potential breaking changes in the TypeScript SDK should be clearly communicated to users.

Action Items

1. 🔴 Address the security concerns related to stripKeyPrefix and audit log validation.
2. 🟡 Document the breaking changes in the TypeScript SDK and consider a major version bump.
3. 💡 Implement the suggested improvements for error handling, test coverage, and documentation.

Once these issues are resolved, the PR will be ready for approval.

[github-actions]

🤖 AI Agent: security-scanner — Security Review for PR: Upstream AzureClaw SDK Patches + GovernanceService Proto

Security Review for PR: Upstream AzureClaw SDK Patches + GovernanceService Proto

---

1. Prompt Injection Defense Bypass

Rating: 🔵 LOW

No direct evidence of prompt injection vulnerabilities was found in this PR. The changes primarily focus on key prefix handling, base64 decoding, and the addition of a governance service proto. None of these changes involve user-controlled inputs being directly used in prompts.

Recommendation:

• Ensure that any user-controlled inputs passed to the GovernanceService are sanitized and validated before being processed.
• Implement strict input validation for PolicyRequest fields like action, resource, and context.

---

2. Policy Engine Circumvention

Rating: 🟠 HIGH

The GovernanceService proto introduces a critical policy evaluation mechanism (EvaluatePolicy) that determines whether an action is permitted. However, there is no evidence in the PR that the implementation of EvaluatePolicy enforces strict validation of the PolicyRequest fields. If an attacker can craft a malicious PolicyRequest (e.g., with invalid or unexpected action or context values), they may bypass policy enforcement.

Attack Vector:

• An attacker could craft a PolicyRequest with malformed or unexpected action or context values to bypass policy checks or cause undefined behavior in the policy engine.

Recommendation:

• Ensure that the EvaluatePolicy implementation includes strict validation of all input fields in PolicyRequest.
• Use a whitelist approach to validate action and resource values against a predefined schema.
• Implement logging and monitoring for all policy evaluation requests to detect anomalies.

---

3. Trust Chain Weaknesses

Rating: 🔴 CRITICAL

The GovernanceService proto includes a TrustQuery RPC to retrieve trust scores for agents. However, there is no mention of how the trust scores are calculated or verified. If the trust score calculation or verification process is flawed, it could lead to a breakdown in the trust chain.

Attack Vector:

• An attacker could manipulate the trust score calculation or exploit a lack of validation in the TrustQuery response to falsely elevate their trust level.

Recommendation:

• Ensure that trust scores are calculated using a secure and auditable process.
• Implement cryptographic signatures for trust score responses to prevent tampering.
• Validate the agent_did field in TrustQuery to ensure it corresponds to a legitimate agent.

---

4. Credential Exposure

Rating: 🔵 LOW

No evidence of credential exposure was found in the PR. The changes do not involve sensitive data handling or logging.

Recommendation:

• Ensure that no sensitive data (e.g., private keys, secrets) is logged or exposed in error messages.

---

5. Sandbox Escape

Rating: 🔵 LOW

The PR does not introduce any changes related to container or process isolation. No evidence of sandbox escape vulnerabilities was found.

Recommendation:

• Ensure that the GovernanceService implementation does not execute untrusted code or commands.

---

6. Deserialization Attacks

Rating: 🟠 HIGH

The AgentIdentity.fromJSON method now uses safeBase64Decode to handle keys with prefixes. However, there is no validation of the deserialized data. If an attacker provides a malicious JSON object, it could lead to unexpected behavior or security issues.

Attack Vector:

• An attacker could craft a malicious JSON object with unexpected or invalid fields to exploit the deserialization process.

Recommendation:

• Validate all fields in the JSON object before deserializing.
• Use a schema validation library to enforce strict type and format checks.

---

7. Race Conditions

Rating: 🔵 LOW

No evidence of race conditions was found in the PR. The changes do not involve concurrent operations or time-of-check-to-time-of-use (TOCTOU) vulnerabilities.

Recommendation:

• Ensure that the GovernanceService implementation is thread-safe and handles concurrent requests securely.

---

8. Supply Chain

Rating: 🟠 HIGH

The PR introduces a dependency on the official agentmesh-sdk instead of the previously vendored version. While this is a positive change, it introduces a potential supply chain risk if the official SDK is compromised.

Attack Vector:

• If the agentmesh-sdk is compromised (e.g., via dependency hijacking or malicious updates), it could introduce vulnerabilities into the project.

Recommendation:

• Use a dependency scanning tool to monitor the agentmesh-sdk for vulnerabilities.
• Pin the dependency to a specific version and review updates before upgrading.
• Consider implementing a package integrity verification mechanism (e.g., hash verification) for the agentmesh-sdk.

---

Summary of Findings

Issue	Rating	Recommendation
Prompt Injection Defense Bypass	🔵 LOW	Validate and sanitize all user inputs to GovernanceService.
Policy Engine Circumvention	🟠 HIGH	Enforce strict validation of PolicyRequest fields and monitor policy evaluations.
Trust Chain Weaknesses	🔴 CRITICAL	Secure trust score calculation and verification with cryptographic measures.
Credential Exposure	🔵 LOW	Ensure no sensitive data is logged or exposed.
Sandbox Escape	🔵 LOW	Ensure no untrusted code execution in GovernanceService.
Deserialization Attacks	🟠 HIGH	Validate JSON objects before deserialization in AgentIdentity.fromJSON.
Race Conditions	🔵 LOW	Ensure thread-safety in GovernanceService.
Supply Chain	🟠 HIGH	Monitor agentmesh-sdk for vulnerabilities and pin dependency versions.

---

Final Recommendation

• Address the CRITICAL and HIGH findings before merging this PR.
• Specifically, focus on securing the trust chain in the GovernanceService and implementing robust input validation for PolicyRequest and deserialization in AgentIdentity.fromJSON.
• Perform a thorough review of the agentmesh-sdk dependency and consider implementing additional supply chain security measures.