From 5c3b9bb6e1b42dbb0a47022709b3257739ee5af3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 20:26:39 +0000 Subject: [PATCH 1/2] fix(storage): require signed auth for restricted storage reads Restricted/owner storage-program reads were authorized against an unsigned, caller-supplied requesterAddress string, so any anonymous caller who named an allowlisted address could read confidential plaintext data. The write path already derives the sender from a verified signature; this brings reads in line. The read requester is now derived solely from an optional signed auth envelope { identity, signature, timestamp } verified over a canonical, replay-bounded message before the ACL check. A caller-supplied address string is never trusted. Public reads stay open (anonymous). Missing or invalid auth resolves to anonymous, so non-public reads fail closed. Covers both read paths: getAccessibleProgram (single reads) and the readReachablePredicate-backed list/search routines. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_018WcfQH7UwvZoHz2tHUyTLk --- .../routines/nodecalls/getStorageProgram.ts | 12 +- .../nodecalls/getStorageProgramAll.ts | 9 +- .../nodecalls/getStorageProgramFields.ts | 9 +- .../nodecalls/getStorageProgramItem.ts | 9 +- .../nodecalls/getStorageProgramsByOwner.ts | 15 +- .../nodecalls/hasStorageProgramField.ts | 9 +- .../nodecalls/searchStoragePrograms.ts | 15 +- .../nodecalls/storageProgramShared.ts | 42 +++-- .../routines/nodecalls/storageReadAuth.ts | 94 +++++++++++ src/libs/network/verifySignature.ts | 79 +++++++++ tests/storageprogram/readauth.test.ts | 158 ++++++++++++++++++ 11 files changed, 387 insertions(+), 64 deletions(-) create mode 100644 src/libs/network/routines/nodecalls/storageReadAuth.ts create mode 100644 tests/storageprogram/readauth.test.ts diff --git a/src/libs/network/routines/nodecalls/getStorageProgram.ts b/src/libs/network/routines/nodecalls/getStorageProgram.ts index 777600da0..004f28a6e 100644 --- a/src/libs/network/routines/nodecalls/getStorageProgram.ts +++ b/src/libs/network/routines/nodecalls/getStorageProgram.ts @@ -9,7 +9,7 @@ import { interface GetStorageProgramData { storageAddress?: unknown - requesterAddress?: unknown + auth?: unknown } /** @@ -20,20 +20,16 @@ interface GetStorageProgramData { * cross-repo contract). * * Enforces ACL via checkReadPermission. Anonymous callers can read public - * programs; restricted/owner programs require requesterAddress. + * programs; restricted/owner programs require a signed `auth` envelope + * proving control of an allowlisted address. */ export default async function getStorageProgram( data: GetStorageProgramData, ): Promise { try { - const requesterAddress = - typeof data?.requesterAddress === "string" - ? data.requesterAddress - : undefined - const result = await getAccessibleProgram( data?.storageAddress, - requesterAddress, + data?.auth, ) if (result.error) { return result.error diff --git a/src/libs/network/routines/nodecalls/getStorageProgramAll.ts b/src/libs/network/routines/nodecalls/getStorageProgramAll.ts index 8611c5891..f9148e299 100644 --- a/src/libs/network/routines/nodecalls/getStorageProgramAll.ts +++ b/src/libs/network/routines/nodecalls/getStorageProgramAll.ts @@ -9,7 +9,7 @@ import { interface GetStorageProgramAllData { storageAddress?: unknown - requesterAddress?: unknown + auth?: unknown } /** @@ -26,14 +26,9 @@ export default async function getStorageProgramAll( data: GetStorageProgramAllData, ): Promise { try { - const requesterAddress = - typeof data?.requesterAddress === "string" - ? data.requesterAddress - : undefined - const result = await getAccessibleProgram( data?.storageAddress, - requesterAddress, + data?.auth, ) if (result.error) return result.error diff --git a/src/libs/network/routines/nodecalls/getStorageProgramFields.ts b/src/libs/network/routines/nodecalls/getStorageProgramFields.ts index bfbe4403e..0c520b144 100644 --- a/src/libs/network/routines/nodecalls/getStorageProgramFields.ts +++ b/src/libs/network/routines/nodecalls/getStorageProgramFields.ts @@ -9,7 +9,7 @@ import { interface GetStorageProgramFieldsData { storageAddress?: unknown - requesterAddress?: unknown + auth?: unknown } /** @@ -22,14 +22,9 @@ export default async function getStorageProgramFields( data: GetStorageProgramFieldsData, ): Promise { try { - const requesterAddress = - typeof data?.requesterAddress === "string" - ? data.requesterAddress - : undefined - const result = await getAccessibleProgram( data?.storageAddress, - requesterAddress, + data?.auth, ) if (result.error) return result.error diff --git a/src/libs/network/routines/nodecalls/getStorageProgramItem.ts b/src/libs/network/routines/nodecalls/getStorageProgramItem.ts index 819f861fc..210aa7632 100644 --- a/src/libs/network/routines/nodecalls/getStorageProgramItem.ts +++ b/src/libs/network/routines/nodecalls/getStorageProgramItem.ts @@ -12,7 +12,7 @@ interface GetStorageProgramItemData { storageAddress?: unknown field?: unknown index?: unknown - requesterAddress?: unknown + auth?: unknown } /** @@ -35,14 +35,9 @@ export default async function getStorageProgramItem( } const rawIndex = Math.trunc(data.index) - const requesterAddress = - typeof data?.requesterAddress === "string" - ? data.requesterAddress - : undefined - const result = await getAccessibleProgram( data?.storageAddress, - requesterAddress, + data?.auth, ) if (result.error) return result.error diff --git a/src/libs/network/routines/nodecalls/getStorageProgramsByOwner.ts b/src/libs/network/routines/nodecalls/getStorageProgramsByOwner.ts index cba90952d..e2eaa6396 100644 --- a/src/libs/network/routines/nodecalls/getStorageProgramsByOwner.ts +++ b/src/libs/network/routines/nodecalls/getStorageProgramsByOwner.ts @@ -8,10 +8,11 @@ import { rpcInternalError, toStorageProgramListItem, } from "./storageProgramShared" +import { readAuthScope, resolveReadRequester } from "./storageReadAuth" interface GetStorageProgramsByOwnerData { owner?: unknown - requesterAddress?: unknown + auth?: unknown limit?: unknown offset?: unknown } @@ -38,11 +39,13 @@ export default async function getStorageProgramsByOwner( } const owner = data.owner - const requesterAddress = - typeof data?.requesterAddress === "string" && - data.requesterAddress.length > 0 - ? data.requesterAddress - : undefined + // Requester derived from a verified signature only — a plain address + // string is never trusted, so restricted programs stay hidden from + // anonymous callers (only public rows survive the SQL ACL filter). + const requesterAddress = await resolveReadRequester( + readAuthScope.owner(owner), + data?.auth, + ) const rawLimit = typeof data?.limit === "number" ? data.limit : 100 const limit = Math.min(Math.max(1, Math.floor(rawLimit)), 200) diff --git a/src/libs/network/routines/nodecalls/hasStorageProgramField.ts b/src/libs/network/routines/nodecalls/hasStorageProgramField.ts index 2565b80c1..406f46451 100644 --- a/src/libs/network/routines/nodecalls/hasStorageProgramField.ts +++ b/src/libs/network/routines/nodecalls/hasStorageProgramField.ts @@ -10,7 +10,7 @@ import { interface HasStorageProgramFieldData { storageAddress?: unknown field?: unknown - requesterAddress?: unknown + auth?: unknown } /** @@ -31,14 +31,9 @@ export default async function hasStorageProgramField( } const field = data.field - const requesterAddress = - typeof data?.requesterAddress === "string" - ? data.requesterAddress - : undefined - const result = await getAccessibleProgram( data?.storageAddress, - requesterAddress, + data?.auth, ) if (result.error) return result.error diff --git a/src/libs/network/routines/nodecalls/searchStoragePrograms.ts b/src/libs/network/routines/nodecalls/searchStoragePrograms.ts index 7b5319d95..e06e805cd 100644 --- a/src/libs/network/routines/nodecalls/searchStoragePrograms.ts +++ b/src/libs/network/routines/nodecalls/searchStoragePrograms.ts @@ -8,6 +8,7 @@ import { rpcInternalError, toStorageProgramListItem, } from "./storageProgramShared" +import { readAuthScope, resolveReadRequester } from "./storageReadAuth" interface SearchStorageProgramsOptions { limit?: unknown @@ -19,7 +20,7 @@ interface SearchStorageProgramsOptions { interface SearchStorageProgramsData { query?: unknown options?: SearchStorageProgramsOptions - requesterAddress?: unknown + auth?: unknown } /** @@ -46,11 +47,13 @@ export default async function searchStoragePrograms( } const query = data.query.trim() - const requesterAddress = - typeof data?.requesterAddress === "string" && - data.requesterAddress.length > 0 - ? data.requesterAddress - : undefined + // Requester derived from a verified signature only — a plain address + // string is never trusted, so restricted programs stay hidden from + // anonymous callers (only public rows survive the SQL ACL filter). + const requesterAddress = await resolveReadRequester( + readAuthScope.search(query), + data?.auth, + ) const opts = data?.options ?? {} const rawLimit = typeof opts.limit === "number" ? opts.limit : 100 diff --git a/src/libs/network/routines/nodecalls/storageProgramShared.ts b/src/libs/network/routines/nodecalls/storageProgramShared.ts index 5eabcd7c3..ce453ebf3 100644 --- a/src/libs/network/routines/nodecalls/storageProgramShared.ts +++ b/src/libs/network/routines/nodecalls/storageProgramShared.ts @@ -3,6 +3,7 @@ import Datasource from "src/model/datasource" import { GCRStorageProgram } from "src/model/entities/GCRv2/GCR_StorageProgram" import { GCRStorageProgramRoutines } from "src/libs/blockchain/gcr/gcr_routines/GCRStorageProgramRoutines" import log from "src/utilities/logger" +import { readAuthScope, resolveReadRequester } from "./storageReadAuth" export type StorageFieldType = | "string" @@ -93,6 +94,13 @@ export async function getStorageProgramRepository() { /** * Validate a storageAddress and fetch the program with ACL enforcement. * + * The requester is derived from an optional signed `auth` envelope only — + * never from a caller-supplied address string. Public programs stay readable + * anonymously; restricted/owner programs require a fresh, valid signature + * proving control of the address (see {@link resolveReadRequester}). Missing + * or invalid auth resolves to anonymous, so non-public reads fail closed with + * the same 403 as an unmatched ACL. + * * Returns either: * - { program } on success, or * - { error: RPCResponse } when the address is invalid, the program is missing @@ -101,7 +109,7 @@ export async function getStorageProgramRepository() { */ export async function getAccessibleProgram( storageAddress: unknown, - requesterAddress?: string, + auth?: unknown, ): Promise<{ program?: GCRStorageProgram; error?: RPCResponse }> { if (typeof storageAddress !== "string" || storageAddress.length === 0) { return { @@ -126,6 +134,14 @@ export async function getAccessibleProgram( return { error: rpcNull() } } + // Bind the ACL check to a verified signature over this storageAddress; + // undefined (anonymous) when auth is absent/invalid so non-public reads + // fail closed. + const requesterAddress = await resolveReadRequester( + readAuthScope.program(storageAddress), + auth, + ) + const hasReadAccess = GCRStorageProgramRoutines.checkReadPermission( program, requesterAddress, @@ -208,7 +224,7 @@ export function requireJsonObject( export interface FieldReadInput { storageAddress?: unknown field?: unknown - requesterAddress?: unknown + auth?: unknown } /** @@ -227,14 +243,14 @@ export interface FieldReadContext { * * The envelope is identical across these handlers: * 1. validate `field` arg (non-empty string) -> 400 / INVALID_REQUEST - * 2. coerce `requesterAddress` to string|undefined - * 3. resolve the program with ACL enforcement (404/403/null already - * handled inside getAccessibleProgram) - * 4. require JSON object data -> 400 / INVALID_FIELD_TYPE - * 5. confirm the requested field exists on the object -> 404 / + * 2. resolve the program with ACL enforcement (404/403/null already + * handled inside getAccessibleProgram; the requester is derived from + * the optional signed `auth` envelope, not a plain address string) + * 3. require JSON object data -> 400 / INVALID_FIELD_TYPE + * 4. confirm the requested field exists on the object -> 404 / * FIELD_NOT_FOUND - * 6. delegate the response shape to the reducer - * 7. catch + log + 500 / INTERNAL_ERROR + * 5. delegate the response shape to the reducer + * 6. catch + log + 500 / INTERNAL_ERROR * * The reducer is the only thing that varies between handlers, so we keep * it tiny: it receives the resolved {field, value, program} and returns @@ -256,15 +272,9 @@ export function withFieldRead( } const field = data.field - const requesterAddress = - typeof data?.requesterAddress === "string" && - data.requesterAddress.length > 0 - ? data.requesterAddress - : undefined - const result = await getAccessibleProgram( data?.storageAddress, - requesterAddress, + data?.auth, ) if (result.error) return result.error diff --git a/src/libs/network/routines/nodecalls/storageReadAuth.ts b/src/libs/network/routines/nodecalls/storageReadAuth.ts new file mode 100644 index 000000000..243fb2511 --- /dev/null +++ b/src/libs/network/routines/nodecalls/storageReadAuth.ts @@ -0,0 +1,94 @@ +import { verifySignatureOverMessage } from "src/libs/network/verifySignature" +import log from "src/utilities/logger" + +/** + * Optional authentication envelope for storage-program reads. + * + * `identity` is the signer in "algorithm:publicKeyHex" form. The public-key + * portion is the ACL address form (it matches `program.owner` / `acl.allowed` + * / group members, which are all the raw sender public key). `signature` is + * over the canonical read message (see {@link readAuthMessage}) and + * `timestamp` (unix ms) bounds replay to a freshness window. + */ +export interface StorageReadAuth { + identity: string + signature: string + timestamp: number +} + +/** + * Freshness window for read-auth signatures (ms). A signature is only + * accepted when its timestamp is within +/- this bound of the node clock, + * limiting how long a captured signature can be replayed. + */ +export const READ_AUTH_MAX_SKEW_MS = 5 * 60 * 1000 + +/** + * Canonical message a caller must sign to prove control of an address for a + * read. Binds the signature to the queried resource (`scope`) and a + * timestamp so it can't be replayed against a different resource or outside + * the freshness window. The SDK must build the identical string when signing. + */ +export function readAuthMessage(scope: string, timestamp: number): string { + return `demos.storageProgram.read:${scope}:${timestamp}` +} + +/** + * Scope builders for the canonical read message, one per read surface so a + * signature for one query can't be reused for another. + */ +export const readAuthScope = { + program: (storageAddress: string) => storageAddress, + owner: (owner: string) => `owner:${owner}`, + search: (query: string) => `search:${query}`, +} + +function parseReadAuth(auth: unknown): StorageReadAuth | undefined { + if (!auth || typeof auth !== "object") return undefined + const a = auth as Record + if (typeof a.identity !== "string" || a.identity.length === 0) { + return undefined + } + if (typeof a.signature !== "string" || a.signature.length === 0) { + return undefined + } + if (typeof a.timestamp !== "number" || !Number.isFinite(a.timestamp)) { + return undefined + } + return { identity: a.identity, signature: a.signature, timestamp: a.timestamp } +} + +/** + * Resolve the authenticated requester for a read. + * + * Returns the verified requester address (raw public-key hex, the ACL address + * form) only when `auth` carries a fresh, valid signature over the canonical + * message for `scope`; otherwise returns undefined (treated as anonymous). + * + * The requester is derived solely from the verified signature — a + * caller-supplied address string is never trusted — so a non-public program + * stays unreadable without cryptographic proof of key ownership. Fails closed + * (undefined) on missing, malformed, stale, or invalid auth. + */ +export async function resolveReadRequester( + scope: string, + auth: unknown, + now: number = Date.now(), +): Promise { + const parsed = parseReadAuth(auth) + if (!parsed) return undefined + + if (Math.abs(now - parsed.timestamp) > READ_AUTH_MAX_SKEW_MS) { + log.debug("[storageReadAuth] Rejected stale/future read-auth timestamp") + return undefined + } + + const result = await verifySignatureOverMessage( + parsed.identity, + parsed.signature, + readAuthMessage(scope, parsed.timestamp), + ) + if (!result.valid || !result.publicKey) return undefined + + return result.publicKey +} diff --git a/src/libs/network/verifySignature.ts b/src/libs/network/verifySignature.ts index 07a80afd9..495d39706 100644 --- a/src/libs/network/verifySignature.ts +++ b/src/libs/network/verifySignature.ts @@ -134,6 +134,85 @@ export async function verifySignature( } } +/** + * Verify a signature over an explicit, caller-supplied message. + * + * Unlike {@link verifySignature} (whose signed message is fixed to the + * identity / public-key form), this verifies a signature over an arbitrary + * canonical message. Used for replay-resistant request authentication where + * the message binds the signer to a specific request. + * + * `identity` must be in "algorithm:publicKeyHex" form. The recovered public + * key (raw hex, no prefix) is returned so callers can match it against stored + * addresses (which use the same raw public-key form as `tx.content.from`). + * + * @param identity - Signer identity, "algorithm:publicKeyHex" + * @param signature - Hex-encoded signature over `message` + * @param message - The exact canonical message that was signed + */ +export async function verifySignatureOverMessage( + identity: string, + signature: string, + message: string, +): Promise { + if (!identity || !signature || !message) { + return { + valid: false, + identity: identity || null, + publicKey: null, + algorithm: null, + error: "Missing identity, signature, or message", + } + } + + const splits = identity.split(":") + if (splits.length < 2 || !SUPPORTED_ALGORITHMS.includes(splits[0])) { + return { + valid: false, + identity, + publicKey: null, + algorithm: null, + error: "Unsupported or malformed identity", + } + } + + const algorithm = splits[0] + const publicKeyHex = splits[1] + + try { + const signatureObj = { + algorithm, + signature: hexToUint8Array(signature), + message: new TextEncoder().encode(message), + publicKey: hexToUint8Array(publicKeyHex), + } as Ed25519SignedObject + + const isValid = await ucrypto.verify(signatureObj) + if (isValid) { + return { valid: true, identity, publicKey: publicKeyHex, algorithm } + } + + return { + valid: false, + identity, + publicKey: publicKeyHex, + algorithm, + error: "Invalid signature", + } + } catch (error) { + log.error( + `[verifySignatureOverMessage] Error verifying signature: ${error}`, + ) + return { + valid: false, + identity, + publicKey: null, + algorithm, + error: `Verification error: ${error}`, + } + } +} + /** * Check if a public key is in the whitelist * diff --git a/tests/storageprogram/readauth.test.ts b/tests/storageprogram/readauth.test.ts new file mode 100644 index 000000000..da7f27588 --- /dev/null +++ b/tests/storageprogram/readauth.test.ts @@ -0,0 +1,158 @@ +import { beforeAll, describe, expect, it, jest } from "@jest/globals" + +// Mock logger (matches the other storageprogram suites) +jest.mock("@/utilities/logger", () => ({ + __esModule: true, + default: { + info: jest.fn(), + debug: jest.fn(), + warning: jest.fn(), + error: jest.fn(), + }, +})) + +// Mock SDK barrel + datasource so importing the routines module is side-effect +// free (no real DB). The demosdk/encryption mock's ucrypto.verify returns true, +// so a well-formed envelope verifies; freshness/parse checks run before it. +jest.mock("@kynesyslabs/demosdk", () => ({ + __esModule: true, + types: {}, + storage: {}, +})) +jest.mock("@/model/datasource", () => ({ + __esModule: true, + default: { getInstance: jest.fn() }, +})) + +let resolveReadRequester: typeof import("src/libs/network/routines/nodecalls/storageReadAuth")["resolveReadRequester"] +let readAuthScope: typeof import("src/libs/network/routines/nodecalls/storageReadAuth")["readAuthScope"] +let READ_AUTH_MAX_SKEW_MS: number +let GCRStorageProgramRoutines: typeof import("src/libs/blockchain/gcr/gcr_routines/GCRStorageProgramRoutines")["GCRStorageProgramRoutines"] + +// An allowlisted address = the raw public-key hex (the ACL address form). +const ALLOWED_HEX = "aa".repeat(32) +const STORAGE_ADDRESS = "stor-" + "0".repeat(40) + +function restrictedProgram() { + return { + storageAddress: STORAGE_ADDRESS, + owner: "bb".repeat(32), + acl: { mode: "restricted", allowed: [ALLOWED_HEX] }, + } as any +} + +function publicProgram() { + return { + storageAddress: STORAGE_ADDRESS, + owner: "bb".repeat(32), + acl: { mode: "public" }, + } as any +} + +// A well-formed, fresh auth envelope naming ALLOWED_HEX. ucrypto.verify is +// mocked to true, so this stands in for a real signature. +function validAuth(scope = readAuthScope.program(STORAGE_ADDRESS)) { + void scope + return { + identity: `ed25519:${ALLOWED_HEX}`, + signature: "cc".repeat(64), + timestamp: Date.now(), + } +} + +beforeAll(async () => { + ;({ resolveReadRequester, readAuthScope, READ_AUTH_MAX_SKEW_MS } = + await import("src/libs/network/routines/nodecalls/storageReadAuth")) + ;({ GCRStorageProgramRoutines } = await import( + "src/libs/blockchain/gcr/gcr_routines/GCRStorageProgramRoutines" + )) +}) + +describe("storage read-auth: resolveReadRequester", () => { + const scope = () => readAuthScope.program(STORAGE_ADDRESS) + + it("returns undefined when no auth envelope is supplied", async () => { + expect(await resolveReadRequester(scope(), undefined)).toBeUndefined() + }) + + it("ignores a caller-supplied address string with no signature", async () => { + // The core of the fix: a bare requesterAddress-style value is never + // trusted for authorization. + expect( + await resolveReadRequester(scope(), { identity: ALLOWED_HEX }), + ).toBeUndefined() + }) + + it("rejects a malformed identity (no algorithm prefix)", async () => { + const auth = { ...validAuth(), identity: ALLOWED_HEX } + expect(await resolveReadRequester(scope(), auth)).toBeUndefined() + }) + + it("rejects a stale timestamp (outside the freshness window)", async () => { + const auth = { + ...validAuth(), + timestamp: Date.now() - (READ_AUTH_MAX_SKEW_MS + 1000), + } + expect(await resolveReadRequester(scope(), auth)).toBeUndefined() + }) + + it("rejects a future timestamp (outside the freshness window)", async () => { + const auth = { + ...validAuth(), + timestamp: Date.now() + (READ_AUTH_MAX_SKEW_MS + 1000), + } + expect(await resolveReadRequester(scope(), auth)).toBeUndefined() + }) + + it("returns the verified public key for a fresh, valid signature", async () => { + const requester = await resolveReadRequester(scope(), validAuth()) + expect(requester).toBe(ALLOWED_HEX) + }) +}) + +describe("storage read-auth: end-to-end ACL enforcement", () => { + const scope = () => readAuthScope.program(STORAGE_ADDRESS) + + it("denies a restricted read with no auth (anonymous)", async () => { + const requester = await resolveReadRequester(scope(), undefined) + expect( + GCRStorageProgramRoutines.checkReadPermission( + restrictedProgram(), + requester, + ), + ).toBe(false) + }) + + it("denies a restricted read that only names an allowed address", async () => { + // Bypass attempt: name the allowlisted address but provide no signature. + const requester = await resolveReadRequester(scope(), { + identity: ALLOWED_HEX, + }) + expect( + GCRStorageProgramRoutines.checkReadPermission( + restrictedProgram(), + requester, + ), + ).toBe(false) + }) + + it("allows a restricted read with a valid signature for an allowed address", async () => { + const requester = await resolveReadRequester(scope(), validAuth()) + expect( + GCRStorageProgramRoutines.checkReadPermission( + restrictedProgram(), + requester, + ), + ).toBe(true) + }) + + it("allows a public read with no auth", async () => { + const requester = await resolveReadRequester(scope(), undefined) + expect( + GCRStorageProgramRoutines.checkReadPermission( + publicProgram(), + requester, + ), + ).toBe(true) + }) +}) From f167335c9013d3b128cd486039a872432387e93c Mon Sep 17 00:00:00 2001 From: shitikyan Date: Wed, 15 Jul 2026 17:44:36 +0400 Subject: [PATCH 2/2] fix(storage): close the read-ACL bypass on the HTTP + gcr_routine surfaces too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #970 fixed the manageNodeCall read path, but the same plaintext storage data is reachable via two other mounted, public surfaces that still trusted an unsigned, caller-supplied requester — so the High-severity bypass stayed open: 1. HTTP routes (`src/features/storageprogram/routes.ts`, mounted at server_rpc.ts). `getRequesterAddress` read the raw `identity` header and passed it straight to the ACL. The auth middleware only verifies a signature when BOTH `identity` and `signature` headers are present; a caller who sends `identity` alone naming an allowlisted address was let through and granted access. Now the requester is the key the middleware VERIFIED for the request (`getAuthContext(req).publicKey`) — an unsigned header sets no context and reads as anonymous. 2. `gcr_routine` (`manageGCRRoutines.ts`, cases getStorageProgram / getStorageProgramsByOwner / searchStoragePrograms). These took the requester from `params[1]`/`params[2]` — a caller-supplied value. Now they derive it from the verified `sender` the RPC layer already authenticated; anonymous (empty sender) resolves to undefined, so restricted programs stay hidden. Both surfaces already had a verified identity available (unlike the nodecall path, which is why #970 needed a per-request signed envelope) — the bug was that they ignored it in favour of an unsigned value. Tests (jest, run in-band): - readauth-surfaces.test.ts (new): getRequesterAddress ignores a raw identity header, returns only the middleware-verified key — on the REAL auth context. - gcrroutine-readauth.test.ts (new): the ACL requester is the verified sender, never the params value; empty sender reads anonymous. - routes.test.ts: the owner-mode test now authenticates via the verified context, plus a new "unverified raw identity header -> 403" bypass case; full suite green (25). SDK follow-up (same as #970): storage-read clients must authenticate the request (identity + signature headers) rather than pass a requesterAddress arg. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/features/storageprogram/routes.ts | 18 +++-- src/libs/network/manageGCRRoutines.ts | 14 +++- .../gcrroutine-readauth.test.ts | 76 +++++++++++++++++++ .../storageprogram/readauth-surfaces.test.ts | 67 ++++++++++++++++ tests/storageprogram/routes.test.ts | 41 +++++++++- 5 files changed, 201 insertions(+), 15 deletions(-) create mode 100644 tests/storageprogram/gcrroutine-readauth.test.ts create mode 100644 tests/storageprogram/readauth-surfaces.test.ts diff --git a/src/features/storageprogram/routes.ts b/src/features/storageprogram/routes.ts index f853dee63..5ac3f1eee 100644 --- a/src/features/storageprogram/routes.ts +++ b/src/features/storageprogram/routes.ts @@ -18,6 +18,7 @@ import log from "@/utilities/logger" import Datasource from "@/model/datasource" import { GCRStorageProgram } from "@/model/entities/GCRv2/GCR_StorageProgram" import { GCRStorageProgramRoutines } from "@/libs/blockchain/gcr/gcr_routines/GCRStorageProgramRoutines" +import { getAuthContext } from "@/libs/network/authContext" // ============================================================================ // Response Types @@ -114,14 +115,15 @@ interface StorageProgramGranularResponse { * behaviour with the SQL ACL filter and other call sites that distinguish * `""` from `undefined`. */ -function getRequesterAddress(req: Request): string | undefined { - const identity = req.headers.get("identity") - if (!identity || identity.length === 0) { - return undefined - } - const splits = identity.split(":") - const candidate = splits.length > 1 ? splits[1] : identity - return candidate && candidate.length > 0 ? candidate : undefined +// Exported for tests. +export function getRequesterAddress(req: Request): string | undefined { + // The requester is the key the auth middleware VERIFIED for this request — + // never the raw `identity` header. The middleware only populates the auth + // context after checking the request signature (and 401s on a bad one), so + // a caller who merely names an allowlisted address in the header, with no + // signature, has no auth context and reads as anonymous. Trusting the raw + // header here was the storage read-ACL bypass. + return getAuthContext(req).publicKey ?? undefined } function getValueType( diff --git a/src/libs/network/manageGCRRoutines.ts b/src/libs/network/manageGCRRoutines.ts index 20ed9d84e..d83634d4c 100644 --- a/src/libs/network/manageGCRRoutines.ts +++ b/src/libs/network/manageGCRRoutines.ts @@ -28,6 +28,14 @@ export default async function manageGCRRoutines( // Handle the payload const { method, params } = payload + // The requester for every storage-program read ACL check is the key the RPC + // layer VERIFIED for this call (`sender`), never a caller-supplied params + // value: naming an allowlisted address without proving key ownership was the + // storage read-ACL bypass. Empty sender (anonymous) resolves to undefined, + // so restricted programs stay hidden unless the caller signed the request. + const verifiedRequester = + typeof sender === "string" && sender.length > 0 ? sender : undefined + switch (method) { // SECTION XM Identity Management @@ -290,7 +298,7 @@ export default async function manageGCRRoutines( // REVIEW: Get storage program by address case "getStorageProgram": { const storageAddress = params[0] - const requesterAddress = params[1] // Optional identity for ACL check + const requesterAddress = verifiedRequester // verified signer, not caller-supplied if (!storageAddress) { response.result = 400 @@ -362,7 +370,7 @@ export default async function manageGCRRoutines( // REVIEW: Get storage programs by owner case "getStorageProgramsByOwner": { const owner = params[0] - const requesterAddress = params[1] // Optional identity for ACL filtering + const requesterAddress = verifiedRequester // verified signer, not caller-supplied const options = params[2] || {} // Optional { limit, offset } if (!owner) { @@ -425,7 +433,7 @@ export default async function manageGCRRoutines( case "searchStoragePrograms": { const query = params[0] const options = params[1] || {} // { limit, offset, exactMatch } - const requesterAddress = params[2] // Optional identity for ACL filtering + const requesterAddress = verifiedRequester // verified signer, not caller-supplied if (!query || (typeof query === "string" && query.trim() === "")) { response.result = 400 diff --git a/tests/storageprogram/gcrroutine-readauth.test.ts b/tests/storageprogram/gcrroutine-readauth.test.ts new file mode 100644 index 000000000..d17c74bcf --- /dev/null +++ b/tests/storageprogram/gcrroutine-readauth.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it, jest, beforeAll } from "@jest/globals" + +// `gcr_routine` storage reads must ACL-filter on the VERIFIED `sender` the RPC +// layer passed in — never a caller-supplied `params` value. Trivial stubs for +// the dispatcher's unrelated import chain; the storage routines are spied so we +// can assert exactly which requester reached the SQL filter. +jest.mock("@/utilities/logger", () => ({ + __esModule: true, + default: { info: jest.fn(), debug: jest.fn(), warning: jest.fn(), error: jest.fn() }, +})) +jest.mock("@kynesyslabs/demosdk/types", () => ({ __esModule: true })) +jest.mock("@/libs/network/server_rpc", () => ({ __esModule: true, emptyResponse: { result: 200, response: null, require_reply: false, extra: null } })) +jest.mock("@/libs/blockchain/gcr/gcr_routines/identityManager", () => ({ __esModule: true, default: {} })) +jest.mock("@/libs/blockchain/gcr/gcr_routines/IncentiveManager", () => ({ __esModule: true, IncentiveManager: {} })) +jest.mock("@/libs/blockchain/gcr/gcr_routines/ensureGCRForUser", () => ({ __esModule: true, default: {} })) +jest.mock("@/features/incentive/referrals", () => ({ __esModule: true, Referrals: {} })) +jest.mock("@/libs/blockchain/gcr/gcr", () => ({ __esModule: true, default: {} })) +jest.mock("@/libs/identity/providers/nomisIdentityProvider", () => ({ __esModule: true, NomisIdentityProvider: {} })) +jest.mock("@/libs/identity/tools/humanpassport", () => ({ __esModule: true, default: {} })) +jest.mock("@/libs/identity/providers/ethosIdentityProvider", () => ({ __esModule: true, EthosIdentityProvider: {} })) +jest.mock("@/libs/communications/broadcastManager", () => ({ __esModule: true, BroadcastManager: {} })) +jest.mock("@/model/entities/GCRv2/GCR_StorageProgram", () => ({ __esModule: true, GCRStorageProgram: class {} })) +jest.mock("@/model/datasource", () => ({ + __esModule: true, + default: { getInstance: async () => ({ getDataSource: () => ({ getRepository: () => ({}) }) }) }, +})) + +const listSpy = jest.fn(async () => []) +const searchSpy = jest.fn(async () => []) +jest.mock("@/libs/blockchain/gcr/gcr_routines/GCRStorageProgramRoutines", () => ({ + __esModule: true, + GCRStorageProgramRoutines: { + getStorageProgramsByOwner: listSpy, + searchStorageProgramsByName: searchSpy, + }, +})) + +let manageGCRRoutines: typeof import("@/libs/network/manageGCRRoutines")["default"] +const VERIFIED = "aa".repeat(32) +const ATTACKER = "bb".repeat(32) // an allowlisted address the caller merely names + +beforeAll(async () => { + ;({ default: manageGCRRoutines } = await import("@/libs/network/manageGCRRoutines")) +}) + +describe("gcr_routine storage reads: ACL requester = verified sender", () => { + it("getStorageProgramsByOwner filters on the verified sender, not params", async () => { + await manageGCRRoutines(VERIFIED, { + method: "getStorageProgramsByOwner", + params: ["owner-x", ATTACKER, {}], + } as never) + // 3rd arg to the routine is the requesterAddress that hits the SQL ACL. + expect(listSpy).toHaveBeenCalled() + expect(listSpy.mock.calls[0][2]).toBe(VERIFIED) + expect(listSpy.mock.calls[0][2]).not.toBe(ATTACKER) + }) + + it("searchStoragePrograms filters on the verified sender, not params", async () => { + await manageGCRRoutines(VERIFIED, { + method: "searchStoragePrograms", + params: ["query", {}, ATTACKER], + } as never) + expect(searchSpy).toHaveBeenCalled() + const opts = searchSpy.mock.calls[0][2] as { requesterAddress?: string } + expect(opts.requesterAddress).toBe(VERIFIED) + expect(opts.requesterAddress).not.toBe(ATTACKER) + }) + + it("an anonymous (empty) sender reads as undefined even if params name an address", async () => { + await manageGCRRoutines("", { + method: "getStorageProgramsByOwner", + params: ["owner-x", ATTACKER, {}], + } as never) + expect(listSpy.mock.calls.at(-1)?.[2]).toBeUndefined() + }) +}) diff --git a/tests/storageprogram/readauth-surfaces.test.ts b/tests/storageprogram/readauth-surfaces.test.ts new file mode 100644 index 000000000..efdbcd319 --- /dev/null +++ b/tests/storageprogram/readauth-surfaces.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it, jest } from "@jest/globals" + +// The two read surfaces #970 did not cover — the HTTP route layer and the +// `gcr_routine` dispatcher — must derive the ACL requester from a VERIFIED +// identity, never a caller-supplied value. These lock that in. +// +// Mock only the heavy/side-effectful imports so the modules load without a DB +// or SDK. authContext is left REAL: the whole point is that the HTTP handler +// reads the middleware-verified context, so the test drives that real context. +jest.mock("@/utilities/logger", () => ({ + __esModule: true, + default: { info: jest.fn(), debug: jest.fn(), warning: jest.fn(), error: jest.fn() }, +})) +jest.mock("@kynesyslabs/demosdk", () => ({ __esModule: true, types: {}, storage: {} })) +jest.mock("@/model/datasource", () => ({ __esModule: true, default: { getInstance: jest.fn() } })) +jest.mock("@/libs/network/bunServer", () => ({ + __esModule: true, + jsonResponse: (body: unknown, status = 200) => ({ body, status }), +})) + +let getRequesterAddress: typeof import("@/features/storageprogram/routes")["getRequesterAddress"] +let setAuthContext: typeof import("@/libs/network/authContext")["setAuthContext"] + +const KEY = "aa".repeat(32) // raw public-key hex — the ACL address form + +beforeAll(async () => { + ;({ getRequesterAddress } = await import("@/features/storageprogram/routes")) + ;({ setAuthContext } = await import("@/libs/network/authContext")) +}) + +function reqNamingAllowed(): Request { + // A caller that NAMES an allowlisted address in the raw `identity` header, + // with no signature — the exact bypass attempt. + return new Request("http://node/storage-program/stor-" + "0".repeat(40), { + headers: { identity: `ed25519:${KEY}` }, + }) +} + +describe("HTTP read surface: getRequesterAddress", () => { + it("ignores the raw identity header — an unsigned caller is anonymous", () => { + // No auth context set (middleware never verified a signature), so even + // though the header names an allowed address, the requester is anonymous. + expect(getRequesterAddress(reqNamingAllowed())).toBeUndefined() + }) + + it("returns the requester only from the middleware-verified context", () => { + const req = reqNamingAllowed() + setAuthContext(req, { + verified: true, + identity: `ed25519:${KEY}`, + publicKey: KEY, + algorithm: "ed25519", + }) + expect(getRequesterAddress(req)).toBe(KEY) + }) + + it("stays anonymous when the context carries no public key", () => { + const req = reqNamingAllowed() + setAuthContext(req, { + verified: false, + identity: null, + publicKey: null, + algorithm: null, + }) + expect(getRequesterAddress(req)).toBeUndefined() + }) +}) diff --git a/tests/storageprogram/routes.test.ts b/tests/storageprogram/routes.test.ts index a34de2f6c..b705aa969 100644 --- a/tests/storageprogram/routes.test.ts +++ b/tests/storageprogram/routes.test.ts @@ -1,6 +1,7 @@ import { beforeAll, beforeEach, describe, expect, it, jest } from "@jest/globals" import { readFileSync } from "fs" import path from "path" +import { setAuthContext } from "@/libs/network/authContext" // Mock logger jest.mock("@/utilities/logger", () => ({ @@ -119,22 +120,37 @@ beforeEach(() => { function makeRequest( urlPath: string, headers: Record = {}, + authenticatedAs?: string, ): Request { - return new Request(`http://localhost:53550${urlPath}`, { + const req = new Request(`http://localhost:53550${urlPath}`, { headers: new Headers(headers), }) + if (authenticatedAs) { + // Simulate the auth middleware after a VERIFIED signature: only then is + // the verified key placed in the request's auth context. A raw + // `identity` header on its own sets nothing and grants nothing — that + // was the read-ACL bypass. + setAuthContext(req, { + verified: true, + identity: `ed25519:${authenticatedAs}`, + publicKey: authenticatedAs, + algorithm: "ed25519", + }) + } + return req } async function callRoute( pattern: string, urlPath: string, headers: Record = {}, + authenticatedAs?: string, ): Promise<{ status: number; body: any }> { const handler = capturedRoutes.get(pattern) if (!handler) { throw new Error(`Route not found: ${pattern}. Available: ${Array.from(capturedRoutes.keys()).join(", ")}`) } - const response = await handler(makeRequest(urlPath, headers)) + const response = await handler(makeRequest(urlPath, headers, authenticatedAs)) const body = await response.json() return { status: response.status, body } } @@ -214,7 +230,7 @@ describe("StorageProgram HTTP Routes", () => { expect(body.errorCode).toBe("PERMISSION_DENIED") }) - it("200: owner-mode program with owner identity", async () => { + it("200: owner-mode program with a verified owner", async () => { mockRepository.findOneBy.mockResolvedValue( entityFixtures.owner_json_program, ) @@ -222,11 +238,28 @@ describe("StorageProgram HTTP Routes", () => { const { status, body } = await callRoute( "/storage-program/*", "/storage-program/stor-owner-only-789", - { identity: "owner_addr_1" }, + {}, + "owner_addr_1", // verified by the middleware, not a raw header ) expect(status).toBe(200) expect(body.success).toBe(true) }) + + it("403: owner-mode named via an unverified raw identity header (bypass closed)", async () => { + mockRepository.findOneBy.mockResolvedValue( + entityFixtures.owner_json_program, + ) + + // The old bypass: name the owner in the `identity` header, no + // signature. With no verified auth context it is anonymous → 403. + const { status, body } = await callRoute( + "/storage-program/*", + "/storage-program/stor-owner-only-789", + { identity: "owner_addr_1" }, + ) + expect(status).toBe(403) + expect(body.errorCode).toBe("PERMISSION_DENIED") + }) }) // =========================================================================