Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| deletionTokenHint = token!.hint | ||
| deletionTokenHash = blake32Hash(deletionToken) | ||
| } else { | ||
| deletionToken = deletionTokenFromHint(state, uploader.keyName, contractID, key) |
There was a problem hiding this comment.
🔴 deletionTokenFromHint called with wrong argument order — uploader.hint is never passed
The call at src/chelonia.ts:2422 passes (state, uploader.keyName, contractID, key), but the function signature at src/utils.ts:1283-1284 is (state, signingKeyName, hint, objectCid, kvKey?). This means contractID is passed as the hint parameter (should be uploader.hint), key (the KV key) is passed as objectCid (should be contractID), and the actual KV key is never passed as kvKey. The uploader.hint value, which is the whole reason this branch exists (uploader.hint != null), is completely ignored, so the wrong key will be looked up and the wrong deletion token will be generated.
| deletionToken = deletionTokenFromHint(state, uploader.keyName, contractID, key) | |
| deletionToken = deletionTokenFromHint(state, uploader.keyName, uploader.hint, contractID, key) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| .sort((a, b) => { | ||
| if (a._notAfterHeight == null) return 1 | ||
| if (b._notAfterHeight == null) return 1 | ||
| return a._notAfterHeight - b._notAfterHeight |
There was a problem hiding this comment.
🔴 findAllKeyIdsByName sort comparator violates antisymmetry, producing non-deterministic ordering
The sort comparator at src/utils.ts:53-56 returns 1 both when a._notAfterHeight == null (line 54) and when b._notAfterHeight == null (line 55). This violates the antisymmetry requirement of comparison functions: when a is revoked and b is active, it returns 1 (a after b); but when a is active and b is revoked, it also returns 1 (a after b). These are contradictory orderings for the same pair, making Array.sort() non-deterministic. Since this function is used in freshDeletionToken (line 1275) to compute a stable index-based hint, and in deletionTokenFromHint (line 1286) to reconstruct the key from that hint, non-deterministic ordering means the hint may point to the wrong key, producing an invalid deletion token.
| .sort((a, b) => { | |
| if (a._notAfterHeight == null) return 1 | |
| if (b._notAfterHeight == null) return 1 | |
| return a._notAfterHeight - b._notAfterHeight | |
| .sort((a, b) => { | |
| if (a._notAfterHeight == null && b._notAfterHeight == null) return 0 | |
| if (a._notAfterHeight == null) return 1 | |
| if (b._notAfterHeight == null) return -1 | |
| return a._notAfterHeight - b._notAfterHeight | |
| }) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const key = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId] | ||
| const token = sign(key, tokenData).slice(0, 24) |
There was a problem hiding this comment.
🔴 sign called with serialized key string instead of deserialized Key object
In both freshDeletionToken (src/utils.ts:1273-1274) and deletionTokenFromHint (src/utils.ts:1291-1292), the key is read directly from rootState.secretKeys[signingKeyId] which stores serialized strings (typed as Record<string, string> at src/types.ts:349). The sign function is then called with this raw string. Every other call site in the codebase deserializes the key first: buildShelterAuthorizationHeader at src/utils.ts:1080 does deserializeKey(SAK), and signedData.ts:126 uses deserializedKey. Missing deserializeKey() will cause sign to receive a string instead of the expected Key object, likely causing a runtime error or incorrect signature.
Prompt for agents
In src/utils.ts, both freshDeletionToken (line 1273-1274) and deletionTokenFromHint (line 1291-1292) need to deserialize the key before calling sign(). Import deserializeKey from @chelonia/crypto (it's already imported at line 2). Then change:
Line 1273-1274 in freshDeletionToken:
const key = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId]
const token = sign(key, tokenData).slice(0, 24)
to:
const serializedKey = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId]
const token = sign(deserializeKey(serializedKey), tokenData).slice(0, 24)
Line 1291-1292 in deletionTokenFromHint:
const key = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId]
const token = sign(key, tokenData).slice(0, 24)
to:
const serializedKey = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId]
const token = sign(deserializeKey(serializedKey), tokenData).slice(0, 24)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
This is a preliminary implementation of KV programmatic tokens, part of the ephemeral messages series.
This is missing more robust checks, a full implementation for files and testing. In addition, this PR doesn't cover changes required in Group Income to make use of these APIs.