Skip to content

Token generation: KV tokens #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
corrideat wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Token generation: KV tokens #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@
"dependencies": {
"@apeleghq/multipart-parser": "1.0.18",
"@apeleghq/rfc8188": "1.0.8",
"@chelonia/crypto": "1.0.1",
"@chelonia/crypto": "file:../crypto/chelonia-crypto-1.0.2.tgz",
"@chelonia/serdes": "1.0.1",
"@sbp/okturtles.data": "0.1.7",
"@sbp/okturtles.eventqueue": "1.2.1",
Expand Down
39 changes: 35 additions & 4 deletions src/chelonia.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
randomHexString,
randomIntFromRange
} from 'turtledash'
import { createCID, multicodes, parseCID } from './functions.js'
import { blake32Hash, createCID, multicodes, parseCID } from './functions.js'
import { Buffer } from 'buffer'
import { NOTIFICATION_TYPE, createClient } from './pubsub/index.js'
import type {
Expand Down Expand Up @@ -69,11 +69,13 @@ import {
checkCanBeGarbageCollected,
clearObject,
collectEventStream,
deletionTokenFromHint,
eventsAfter,
findForeignKeysByContractID,
findKeyIdByName,
findRevokedKeyIdsByName,
findSuitableSecretKeyId,
freshDeletionToken,
getContractIDfromKeyId,
handleFetchResult,
reactiveClearObject
Expand Down Expand Up @@ -2331,14 +2333,16 @@ export default sbp('sbp/selectors/register', {
encryptionKeyId,
signingKeyId,
maxAttempts,
onconflict
onconflict,
uploader
}: {
ifMatch?: string;
innerSigningKeyId?: string | null | undefined;
encryptionKeyId?: string | null | undefined;
signingKeyId: string;
maxAttempts?: number | null | undefined;
onconflict?: ChelKvOnConflictCallback | null | undefined;
uploader?: { contractID: string, keyName: string, hint?: string };
}
) {
maxAttempts = maxAttempts ?? 3
Expand Down Expand Up @@ -2403,6 +2407,22 @@ export default sbp('sbp/selectors/register', {
return true
}

let deletionToken: string | undefined
let deletionTokenHint: string | undefined
let deletionTokenHash: string | undefined

if (uploader) {
const state = sbp('chelonia/contract/state', uploader.contractID)
if (uploader.hint == null) {
const token = freshDeletionToken(state, uploader.keyName, contractID, key)
deletionToken = token!.token
deletionTokenHint = token!.hint
deletionTokenHash = blake32Hash(deletionToken)
} else {
deletionToken = deletionTokenFromHint(state, uploader.keyName, contractID, key)
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.

Suggested change
deletionToken = deletionTokenFromHint(state, uploader.keyName, contractID, key)
deletionToken = deletionTokenFromHint(state, uploader.keyName, uploader.hint, contractID, key)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
}

for (;;) {
if (data !== undefined) {
const serializedData = outputEncryptedOrUnencryptedMessage.call(this, {
Expand All @@ -2415,8 +2435,19 @@ export default sbp('sbp/selectors/register', {
})
response = await this.config.fetch(url, {
headers: new Headers([
['authorization', buildShelterAuthorizationHeader.call(this, contractID)],
['if-match', ifMatch || '""']
['authorization', deletionToken && !deletionTokenHash
? `bearer ${deletionToken}`
: buildShelterAuthorizationHeader.call(this, contractID)
],
['if-match', ifMatch || '""'],
...((deletionTokenHash
? [['shelter-deletion-token-digest', deletionTokenHash]]
: []) as [string, string][]
),
...((deletionTokenHint
? [['shelter-deletion-token-hint', deletionTokenHint]]
: []) as [string, string][]
)
]),
method: 'POST',
body: JSON.stringify(serializedData),
Expand Down
32 changes: 27 additions & 5 deletions src/files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { has } from 'turtledash'
import type { Secret } from './Secret.js'
import { blake32Hash, createCID, createCIDfromStream, multicodes } from './functions.js'
import { ChelFileManifest, CheloniaContext } from './types.js'
import { buildShelterAuthorizationHeader } from './utils.js'
import { buildShelterAuthorizationHeader, freshDeletionToken } from './utils.js'

// Snippet from <https://github.com/WebKit/standards-positions/issues/24#issuecomment-1181821440>
// Node.js supports request streams, but also this check isn't meant for Node.js
Expand Down Expand Up @@ -291,7 +291,13 @@ export default sbp('sbp/selectors/register', {
this: CheloniaContext,
chunks: Blob | Blob[],
manifestOptions: ChelFileManifest,
{ billableContractID }: { billableContractID?: string } = {}
{
billableContractID,
uploader
}: {
billableContractID?: string,
uploader?: { contractID: string, keyName: string }
} = {}
) {
if (!Array.isArray(chunks)) chunks = [chunks]
const chunkDescriptors: Promise<[number, string]>[] = []
Expand Down Expand Up @@ -357,7 +363,19 @@ export default sbp('sbp/selectors/register', {
.join('')
const stream = encodeMultipartMessage(boundary, transferParts)

const deletionToken = 'deletionToken' + generateSalt()
let deletionToken: string
let deletionTokenHint: string | undefined

if (!uploader) {
deletionToken = 'deletionToken' + generateSalt()
} else {
// TODO: state, CID
const state = sbp('chelonia/contract/state', uploader.contractID)
const token = freshDeletionToken(state, uploader.keyName, 'placeholder-cid')
deletionToken = token!.token
deletionTokenHint = token!.hint
}

const deletionTokenHash = blake32Hash(deletionToken)

const uploadResponse = await this.config.fetch(`${this.config.connectionURL}/file`, {
Expand All @@ -369,7 +387,10 @@ export default sbp('sbp/selectors/register', {
? [['authorization', buildShelterAuthorizationHeader.call(this, billableContractID)]]
: []) as [string, string][]),
['content-type', `multipart/form-data; boundary=${boundary}`],
['shelter-deletion-token-digest', deletionTokenHash]
['shelter-deletion-token-digest', deletionTokenHash],
...((deletionTokenHint
? [['shelter-deletion-token-hint', deletionTokenHint]]
: []) as [string, string][])
]),
duplex: 'half'
} as unknown as Request)
Expand All @@ -380,7 +401,8 @@ export default sbp('sbp/selectors/register', {
manifestCid: await uploadResponse.text(),
downloadParams: cipherHandler.downloadParams
},
delete: deletionToken
delete: deletionToken,
hint: deletionTokenHint
}
},
'chelonia/fileDownload': async function (
Expand Down
45 changes: 45 additions & 0 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,20 @@ const MAX_EVENTS_AFTER = Number.parseInt(process.env.MAX_EVENTS_AFTER || '', 10)

const copiedExistingData = Symbol('copiedExistingData')

export const findAllKeyIdsByName = (
state: ChelContractState,
name: string
): string[] | null | undefined =>
state._vm?.authorizedKeys &&
Object.values(state._vm.authorizedKeys)
.filter((k) => k.name === name)
.sort((a, b) => {
if (a._notAfterHeight == null) return 1
if (b._notAfterHeight == null) return 1
return a._notAfterHeight - b._notAfterHeight
Comment on lines +53 to +56
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.

Suggested change
.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
})
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

})
.map(k => k.id)

export const findKeyIdByName = (
state: ChelContractState,
name: string
Expand Down Expand Up @@ -1248,3 +1262,34 @@ export const updateKey = (key: ChelContractKey, updatedKey: ChelContractKey): Ch
...(updatedKey.meta ? { meta: updatedKey.meta } : {})
}
}
export const freshDeletionToken = (
state: ChelContractState, signingKeyName: string, objectCid: string, kvKey?: string
): { token: string, hint: string } | undefined => {
const signingKeyId = findKeyIdByName(state, signingKeyName)
if (!signingKeyId) return

const tokenData = `deletionToken/${objectCid}${kvKey ? `/${kvKey}` : ''}`

const key = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId]
const token = sign(key, tokenData).slice(0, 24)
Comment on lines +1273 to +1274
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const hint = findAllKeyIdsByName(state, signingKeyName)?.findIndex((id) => id === signingKeyId)

return {
token,
hint: String(hint)
}
}

export const deletionTokenFromHint = (
state: ChelContractState, signingKeyName: string, hint: string, objectCid: string, kvKey?: string
): string | undefined => {
const signingKeyId = findAllKeyIdsByName(state, signingKeyName)?.[Number(hint)]
if (!signingKeyId) return

const tokenData = `deletionToken/${objectCid}${kvKey ? `/${kvKey}` : ''}`

const key = (sbp('chelonia/rootState') as ChelRootState).secretKeys[signingKeyId]
const token = sign(key, tokenData).slice(0, 24)

return token
}
Loading