Skip to content

Commit 9bfad8c

Browse files
fotescodevclaude
andcommitted
Add comprehensive NatSpec docs, security tests, and CI tooling
Security assessment improvements based on Trail of Bits guidelines: - Add complete NatSpec documentation to KYAIdentityResolver and KYACapabilityResolver contracts including all functions, events, errors, and state variables - Add 5 new test cases: whitelist toggle behavior, admin transfer overwrites, multiple capabilities per identity, attester removal doesn't affect existing attestations, cross-schema refUID validation - Add Slither static analysis GitHub Actions workflow - Add SDK schema sync script to auto-populate UIDs from deployments - Add hardhat-gas-reporter for gas benchmarking - Document permissionless mode security risks in README - Fix hardhat config to handle malformed private keys gracefully Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f75f3de commit 9bfad8c

9 files changed

Lines changed: 540 additions & 8 deletions

File tree

.github/workflows/security.yml

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
name: Security Analysis
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
paths:
8+
- 'packages/contracts/**'
9+
pull_request:
10+
paths:
11+
- 'packages/contracts/**'
12+
13+
jobs:
14+
slither:
15+
name: Slither Static Analysis
16+
runs-on: ubuntu-latest
17+
permissions:
18+
contents: read
19+
security-events: write
20+
21+
steps:
22+
- name: Checkout repository
23+
uses: actions/checkout@v4
24+
with:
25+
submodules: recursive
26+
27+
- name: Setup Node.js
28+
uses: actions/setup-node@v4
29+
with:
30+
node-version: '20'
31+
cache: 'npm'
32+
33+
- name: Install dependencies
34+
run: npm ci
35+
working-directory: packages/contracts
36+
37+
- name: Compile contracts
38+
run: npm run compile
39+
working-directory: packages/contracts
40+
41+
- name: Run Slither
42+
uses: crytic/slither-action@v0.4.0
43+
id: slither
44+
with:
45+
target: packages/contracts
46+
sarif: results.sarif
47+
fail-on: high
48+
slither-args: --filter-paths "node_modules|test" --exclude-dependencies
49+
50+
- name: Upload Slither SARIF report
51+
uses: actions/upload-artifact@v4
52+
if: always()
53+
with:
54+
name: slither-sarif-report
55+
path: results.sarif
56+
retention-days: 30
57+
58+
- name: Upload SARIF to GitHub Security tab
59+
uses: github/codeql-action/upload-sarif@v3
60+
if: always()
61+
with:
62+
sarif_file: results.sarif

README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,23 @@ The deploy script will:
249249
- **2-step admin transfer** — admin privileges require `transferAdmin` + `acceptAdmin` to prevent accidental transfers
250250
- **Parent validation** — capabilities are rejected if the referenced identity is revoked or expired
251251

252+
### Permissionless Mode Considerations
253+
254+
When `whitelistEnabled=false` on the `KYAIdentityResolver`, anyone can create identity attestations. This permissionless mode carries important risks:
255+
256+
**Risks:**
257+
- **Spam attestations** — Malicious actors can flood the system with garbage attestations, consuming on-chain storage and indexer resources
258+
- **Agent address squatting** — Bad actors may claim agent addresses before legitimate owners, blocking future registrations (since one identity per agent is enforced)
259+
- **DoS via mapping pollution** — Excessive attestations can degrade lookup performance and increase costs for legitimate users
260+
261+
**Mitigations:**
262+
- Keep `whitelistEnabled=true` for production deployments
263+
- Only whitelist trusted attesters who have been vetted
264+
- Implement rate limiting at the application layer (e.g., in your backend or SDK wrapper)
265+
- Monitor attestation patterns for suspicious activity
266+
267+
**Recommendation:** Always enable the attester whitelist for mainnet deployments. Permissionless mode should only be used for testing or controlled environments.
268+
252269
### Reporting Vulnerabilities
253270

254271
This is alpha software. **Do not use in production.**

packages/contracts/contracts/KYACapabilityResolver.sol

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,49 @@ import {SchemaResolver} from "@ethereum-attestation-service/eas-contracts/contra
55
import {IEAS, Attestation} from "@ethereum-attestation-service/eas-contracts/contracts/IEAS.sol";
66

77
/// @title KYACapabilityResolver
8-
/// @notice Resolver for KYA-Capability attestations. Validates that the referenced
9-
/// parent attestation is a valid, non-revoked, non-expired KYA-Identity.
8+
/// @author KYA Protocol
9+
/// @notice Resolver for KYA-Capability attestations that validates parent identity references
10+
/// @dev Implements SchemaResolver to enforce capability→identity linking.
11+
/// Key invariants:
12+
/// - Every capability must reference a valid KYA-Identity via refUID
13+
/// - Parent identity must use the correct schema (identitySchemaUID)
14+
/// - Parent identity must not be revoked
15+
/// - Parent identity must not be expired
16+
/// This is a view-only resolver (no state modifications in onAttest)
1017
contract KYACapabilityResolver is SchemaResolver {
18+
/// @notice The schema UID that parent identity attestations must match
19+
/// @dev Set at deployment and cannot be changed (immutable)
1120
bytes32 public immutable identitySchemaUID;
1221

22+
/// @notice Thrown when refUID is zero or parent schema doesn't match identity schema
1323
error InvalidParentIdentity();
24+
25+
/// @notice Thrown when the parent identity attestation has been revoked
1426
error ParentIdentityRevoked();
27+
28+
/// @notice Thrown when the parent identity attestation has expired
1529
error ParentIdentityExpired();
1630

31+
/// @notice Initializes the resolver with EAS address and identity schema reference
32+
/// @param eas The EAS contract address this resolver integrates with
33+
/// @param _identitySchemaUID The schema UID that valid parent identities must have
1734
constructor(
1835
IEAS eas,
1936
bytes32 _identitySchemaUID
2037
) SchemaResolver(eas) {
2138
identitySchemaUID = _identitySchemaUID;
2239
}
2340

41+
/// @notice Validates a capability attestation references a valid parent identity
42+
/// @dev Called by EAS during attestation creation. This is a view function
43+
/// (no state modifications). Validation order:
44+
/// 1. Require non-zero refUID (must reference parent)
45+
/// 2. Fetch parent attestation from EAS
46+
/// 3. Validate parent uses identity schema
47+
/// 4. Validate parent is not revoked
48+
/// 5. Validate parent is not expired
49+
/// @param attestation The capability attestation being created
50+
/// @return True if attestation should be accepted, reverts otherwise
2451
function onAttest(
2552
Attestation calldata attestation,
2653
uint256 /* value */
@@ -54,6 +81,10 @@ contract KYACapabilityResolver is SchemaResolver {
5481
return true;
5582
}
5683

84+
/// @notice Handles capability attestation revocation
85+
/// @dev Always returns true - no additional validation needed for revocation.
86+
/// Capabilities can be freely revoked by their attesters.
87+
/// @return True to allow the revocation to proceed
5788
function onRevoke(
5889
Attestation calldata,
5990
uint256 /* value */

packages/contracts/contracts/KYAIdentityResolver.sol

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,30 +5,87 @@ import {SchemaResolver} from "@ethereum-attestation-service/eas-contracts/contra
55
import {IEAS, Attestation} from "@ethereum-attestation-service/eas-contracts/contracts/IEAS.sol";
66

77
/// @title KYAIdentityResolver
8-
/// @notice Resolver for KYA-Identity attestations. Enforces one identity per agent
9-
/// address and validates owner is non-zero. Supports optional attester whitelist.
8+
/// @author KYA Protocol
9+
/// @notice Resolver for KYA-Identity attestations that enforces identity registration rules
10+
/// @dev Implements SchemaResolver to validate attestations during creation and revocation.
11+
/// Key invariants:
12+
/// - One identity per agent address (prevents duplicate registrations)
13+
/// - Non-zero owner and agent addresses required
14+
/// - Optional attester whitelist for permissioned mode
15+
/// - 2-step admin transfer to prevent accidental lockout
1016
contract KYAIdentityResolver is SchemaResolver {
17+
/// @notice Maps agent addresses to their identity attestation UIDs
18+
/// @dev Zero value indicates no identity registered for that agent
1119
mapping(address => bytes32) public agentToIdentity;
20+
21+
/// @notice Maps addresses to their attester authorization status
22+
/// @dev Only checked when whitelistEnabled is true
1223
mapping(address => bool) public authorizedAttesters;
24+
25+
/// @notice Current admin address with privileged access
1326
address public admin;
27+
28+
/// @notice Pending admin for 2-step transfer process
1429
address public pendingAdmin;
30+
31+
/// @notice Whether attester whitelist is enforced
32+
/// @dev When false, any address can create attestations (permissionless mode)
1533
bool public whitelistEnabled;
1634

35+
/// @notice Emitted when a new agent identity is registered
36+
/// @param agent The agent address that was registered
37+
/// @param uid The attestation UID for the identity
1738
event IdentityRegistered(address indexed agent, bytes32 uid);
39+
40+
/// @notice Emitted when an agent identity is revoked
41+
/// @param agent The agent address that was revoked
42+
/// @param uid The attestation UID that was revoked
1843
event IdentityRevoked(address indexed agent, bytes32 uid);
44+
45+
/// @notice Emitted when an attester is added to the whitelist
46+
/// @param attester The address added as authorized attester
1947
event AttesterAdded(address indexed attester);
48+
49+
/// @notice Emitted when an attester is removed from the whitelist
50+
/// @param attester The address removed from authorized attesters
2051
event AttesterRemoved(address indexed attester);
52+
53+
/// @notice Emitted when whitelist enforcement is toggled
54+
/// @param enabled The new whitelist state
2155
event WhitelistToggled(bool enabled);
56+
57+
/// @notice Emitted when admin transfer is initiated
58+
/// @param currentAdmin The current admin initiating the transfer
59+
/// @param pendingAdmin The proposed new admin
2260
event AdminTransferStarted(address indexed currentAdmin, address indexed pendingAdmin);
61+
62+
/// @notice Emitted when admin transfer is completed
63+
/// @param previousAdmin The outgoing admin
64+
/// @param newAdmin The new admin who accepted the transfer
2365
event AdminTransferred(address indexed previousAdmin, address indexed newAdmin);
2466

67+
/// @notice Thrown when a non-whitelisted attester tries to create attestation
2568
error UnauthorizedAttester();
69+
70+
/// @notice Thrown when trying to register an identity for an already-registered agent
2671
error AgentAlreadyRegistered();
72+
73+
/// @notice Thrown when owner address is zero
2774
error InvalidOwnerAddress();
75+
76+
/// @notice Thrown when agent address is zero
2877
error InvalidAgentAddress();
78+
79+
/// @notice Thrown when caller is not the admin
2980
error OnlyAdmin();
81+
82+
/// @notice Thrown when caller is not the pending admin
3083
error OnlyPendingAdmin();
3184

85+
/// @notice Initializes the resolver with EAS address and admin configuration
86+
/// @param eas The EAS contract address this resolver integrates with
87+
/// @param _admin The initial admin address for privileged operations
88+
/// @param _whitelistEnabled Whether to enforce attester whitelist from deployment
3289
constructor(
3390
IEAS eas,
3491
address _admin,
@@ -38,11 +95,21 @@ contract KYAIdentityResolver is SchemaResolver {
3895
whitelistEnabled = _whitelistEnabled;
3996
}
4097

98+
/// @dev Restricts function access to the current admin
4199
modifier onlyAdmin() {
42100
if (msg.sender != admin) revert OnlyAdmin();
43101
_;
44102
}
45103

104+
/// @notice Validates and registers a new agent identity attestation
105+
/// @dev Called by EAS during attestation creation. Validation order:
106+
/// 1. Check attester whitelist (if enabled)
107+
/// 2. Validate agent address is non-zero
108+
/// 3. Validate owner address is non-zero
109+
/// 4. Ensure agent doesn't already have an identity
110+
/// 5. Record agent→UID mapping and emit event
111+
/// @param attestation The attestation being created, containing encoded identity data
112+
/// @return True if attestation should be accepted, reverts otherwise
46113
function onAttest(
47114
Attestation calldata attestation,
48115
uint256 /* value */
@@ -86,6 +153,11 @@ contract KYAIdentityResolver is SchemaResolver {
86153
return true;
87154
}
88155

156+
/// @notice Cleans up state when an identity attestation is revoked
157+
/// @dev Called by EAS during attestation revocation. Clears the agent→UID mapping
158+
/// to allow the agent address to register a new identity if needed.
159+
/// @param attestation The attestation being revoked
160+
/// @return True to allow the revocation to proceed
89161
function onRevoke(
90162
Attestation calldata attestation,
91163
uint256 /* value */
@@ -104,26 +176,43 @@ contract KYAIdentityResolver is SchemaResolver {
104176
return true;
105177
}
106178

179+
/// @notice Adds an address to the authorized attesters whitelist
180+
/// @dev Only callable by admin. Has no effect if whitelist is disabled.
181+
/// @param attester The address to authorize as an attester
107182
function addAuthorizedAttester(address attester) external onlyAdmin {
108183
authorizedAttesters[attester] = true;
109184
emit AttesterAdded(attester);
110185
}
111186

187+
/// @notice Removes an address from the authorized attesters whitelist
188+
/// @dev Only callable by admin. Does not affect existing attestations.
189+
/// @param attester The address to remove from authorized attesters
112190
function removeAuthorizedAttester(address attester) external onlyAdmin {
113191
authorizedAttesters[attester] = false;
114192
emit AttesterRemoved(attester);
115193
}
116194

195+
/// @notice Toggles the attester whitelist enforcement
196+
/// @dev Only callable by admin. When disabled, any address can create attestations.
197+
/// WARNING: Disabling whitelist enables permissionless mode - use with caution.
198+
/// @param enabled True to enforce whitelist, false for permissionless mode
117199
function setWhitelistEnabled(bool enabled) external onlyAdmin {
118200
whitelistEnabled = enabled;
119201
emit WhitelistToggled(enabled);
120202
}
121203

204+
/// @notice Initiates a 2-step admin transfer
205+
/// @dev Only callable by current admin. The new admin must call acceptAdmin() to complete.
206+
/// Calling this again with a different address overwrites the pending admin.
207+
/// @param newAdmin The address to transfer admin rights to
122208
function transferAdmin(address newAdmin) external onlyAdmin {
123209
pendingAdmin = newAdmin;
124210
emit AdminTransferStarted(admin, newAdmin);
125211
}
126212

213+
/// @notice Completes the 2-step admin transfer
214+
/// @dev Only callable by the pending admin set via transferAdmin().
215+
/// Resets pendingAdmin to zero after successful transfer.
127216
function acceptAdmin() external {
128217
if (msg.sender != pendingAdmin) revert OnlyPendingAdmin();
129218
emit AdminTransferred(admin, pendingAdmin);

packages/contracts/hardhat.config.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import { HardhatUserConfig } from "hardhat/config";
22
import "@nomicfoundation/hardhat-toolbox";
3+
import "hardhat-gas-reporter";
34
import "dotenv/config";
45

6+
// Handle private key with or without 0x prefix, trim whitespace
7+
// Only use if it's exactly 64 hex characters (32 bytes)
8+
const rawKey = process.env.DEPLOYER_PRIVATE_KEY?.trim().replace(/^0x/, "") || "";
9+
const privateKey = rawKey.length === 64 && /^[0-9a-fA-F]+$/.test(rawKey) ? rawKey : "";
10+
511
const config: HardhatUserConfig = {
612
solidity: {
713
compilers: [
@@ -25,11 +31,19 @@ const config: HardhatUserConfig = {
2531
baseSepolia: {
2632
url: process.env.BASE_SEPOLIA_RPC_URL || "https://sepolia.base.org",
2733
chainId: 84532,
28-
accounts: process.env.DEPLOYER_PRIVATE_KEY
29-
? [process.env.DEPLOYER_PRIVATE_KEY]
30-
: [],
34+
accounts: privateKey ? [privateKey] : [],
3135
},
3236
},
37+
gasReporter: {
38+
enabled: process.env.REPORT_GAS === "true",
39+
currency: "USD",
40+
L2: "base",
41+
L2Etherscan: process.env.BASESCAN_API_KEY,
42+
coinmarketcap: process.env.COINMARKETCAP_API_KEY,
43+
reportFormat: "terminal",
44+
showMethodSig: true,
45+
showTimeSpent: true,
46+
},
3347
};
3448

3549
export default config;

packages/contracts/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"@nomicfoundation/hardhat-toolbox": "^5.0.0",
1414
"dotenv": "^16.4.0",
1515
"hardhat": "^2.22.0",
16+
"hardhat-gas-reporter": "^1.0.10",
1617
"ts-node": "^10.9.0",
1718
"typescript": "^5.4.0"
1819
},

0 commit comments

Comments
 (0)