From 7c23b7bce69dc462f531f1af3d20d28a237ae7ea Mon Sep 17 00:00:00 2001 From: Denis <61563365+dnsi0@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:18:40 +0300 Subject: [PATCH 1/3] stop service as node admin --- docs/API.md | 18 +++- docs/services.md | 2 +- src/components/core/admin/adminHandler.ts | 66 ++++++++----- src/components/core/service/stopService.ts | 32 ++++++- src/test/unit/service/serviceHandlers.test.ts | 93 +++++++++++++++++++ 5 files changed, 178 insertions(+), 33 deletions(-) diff --git a/docs/API.md b/docs/API.md index 6be6f7ad8..336862794 100644 --- a/docs/API.md +++ b/docs/API.md @@ -2616,9 +2616,12 @@ provided. #### Description -Tear down the service container and network. Owner-gated. The paid reservation is kept until -`expiresAt`; optional `release: true` ends the paid window now so the expiry sweep frees it -instead — no refund, no restart. +Tear down the service container and network. Owner-gated, with one exception: a **node +admin** (an address in `ALLOWED_ADMINS` or on an `ALLOWED_ADMINS_LIST` access list) may stop +**any** service on the node by signing as itself in `consumerAddress` — the operator does not +need the tenant's key. The paid reservation is kept until `expiresAt`; optional +`release: true` ends the paid window now so the expiry sweep frees it instead — no refund, +no restart. #### Request Body @@ -2636,6 +2639,15 @@ instead — no refund, no restart. The `ServiceJob` with `status: 70` (Stopped). +#### Response (400) + +No such service. An admin caller gets this too when the `serviceId` does not exist on the +node at all; a non-admin caller gets it for any service it does not own. + +#### Response (401) + +Missing/invalid auth, or `consumerAddress` is neither the service owner nor a node admin. + --- ### `HTTP` GET /api/services/serviceStreamableLogs diff --git a/docs/services.md b/docs/services.md index 00e68036e..f8b31714e 100644 --- a/docs/services.md +++ b/docs/services.md @@ -34,7 +34,7 @@ and `signature` as query parameters (or an auth-token `Authorization` header). | `SERVICE_LIST` | `/api/services/serviceList` | GET | Node-wide service listing — authenticated, **not** owner-scoped. Default: only services currently holding a resource reservation; `status=` filters to one specific status, `includeAllStatuses=true` returns everything, `fromTimestamp` keeps services created at/after that moment. Output is listing-sanitized (no `userData`, no `dockerCmd`/`dockerEntrypoint`, no Dockerfile) but keeps user `metadata` | | `SERVICE_EXTEND` | `/api/services/serviceExtend` | POST | Pay to push the expiry further out | | `SERVICE_RESTART` | `/api/services/serviceRestart` | POST | Recreate the container (no extra charge); asynchronous like start — returns once the job is `Restarting`, poll `serviceStatus`. Optionally restart on a **new image spec** (bug-fix flow) — see below | -| `SERVICE_STOP` | `/api/services/serviceStop` | POST | Tear down the container; the paid resource reservation (cpu/ram/gpu + host ports) is kept until `expiresAt`, so the service can be restarted anytime on the same endpoints. `release: true` ends the paid window now and frees it instead (no refund, no restart) | +| `SERVICE_STOP` | `/api/services/serviceStop` | POST | Tear down the container; the paid resource reservation (cpu/ram/gpu + host ports) is kept until `expiresAt`, so the service can be restarted anytime on the same endpoints. `release: true` ends the paid window now and frees it instead (no refund, no restart). Callable by the owner **or** by a node admin (`ALLOWED_ADMINS` / `ALLOWED_ADMINS_LIST`), who may stop any service on the node | | `SERVICE_GET_TEMPLATES` | `/api/services/serviceTemplates` | GET | List operator-published service templates | | `SERVICE_GET_STREAMABLE_LOGS` | `/api/services/serviceStreamableLogs` | GET | Stream the container's live stdout/stderr logs — authenticated, owner-scoped; available while `Running` or `Error`; optional `since` to skip history | diff --git a/src/components/core/admin/adminHandler.ts b/src/components/core/admin/adminHandler.ts index ed90138ab..feae00a39 100644 --- a/src/components/core/admin/adminHandler.ts +++ b/src/components/core/admin/adminHandler.ts @@ -15,6 +15,46 @@ import { CommonValidation } from '../../../utils/validators.js' import { CORE_LOGGER } from '../../../utils/logging/common.js' import { normalizeCommandAddresses } from '../../../utils/evmAddress.js' +// Membership test for the node's admin set: the ALLOWED_ADMINS address list first, then +// each configured admin access list (ALLOWED_ADMINS_LIST), per chain. Says nothing about +// authentication — the caller must have already proven it owns `address` (signature or +// auth token). Exported because handlers outside the admin family (SERVICE_STOP) also +// grant the node operator a privileged path and must not re-implement these checks. +export async function isAllowedAdminAddress( + allowedAdmins: { addresses: string[]; accessLists: any } | null | undefined, + address: string +): Promise { + if (!allowedAdmins || !address) { + return false + } + const { addresses, accessLists } = allowedAdmins + const isListedAddress = await checkSingleCredential( + { type: CREDENTIALS_TYPES.ADDRESS, values: addresses }, + address, + null + ) + if (isListedAddress) { + return true + } + if (accessLists) { + for (const chainId of Object.keys(accessLists)) { + const isOnAccessList = await checkSingleCredential( + { + type: CREDENTIALS_TYPES.ACCESS_LIST, + chainId: parseInt(chainId), + accessList: accessLists[chainId] + }, + address, + null + ) + if (isOnAccessList) { + return true + } + } + } + return false +} + export abstract class AdminCommandHandler extends BaseHandler implements IValidateAdminCommandHandler @@ -69,33 +109,9 @@ export abstract class AdminCommandHandler } } try { - const allowedAdmins = oceanNode.getAdminAddresses() - - const { addresses, accessLists } = allowedAdmins - let allowed = await checkSingleCredential( - { type: CREDENTIALS_TYPES.ADDRESS, values: addresses }, - address, - null - ) - if (allowed) { + if (await isAllowedAdminAddress(oceanNode.getAdminAddresses(), address)) { return { valid: true, error: '' } } - if (accessLists) { - for (const chainId of Object.keys(accessLists)) { - allowed = await checkSingleCredential( - { - type: CREDENTIALS_TYPES.ACCESS_LIST, - chainId: parseInt(chainId), - accessList: accessLists[chainId] - }, - address, - null - ) - if (allowed) { - return { valid: true, error: '' } - } - } - } const errorMsg = `The address which signed the message is not on the allowed admins list. Therefore signature ${signature} is rejected` CORE_LOGGER.logMessage(errorMsg) diff --git a/src/components/core/service/stopService.ts b/src/components/core/service/stopService.ts index 161423569..ef391218f 100644 --- a/src/components/core/service/stopService.ts +++ b/src/components/core/service/stopService.ts @@ -9,6 +9,7 @@ import { buildInvalidRequestMessage } from '../../httpRoutes/validateCommands.js' import { CORE_LOGGER } from '../../../utils/logging/common.js' +import { isAllowedAdminAddress } from '../admin/adminHandler.js' import { findServiceJobAndEngine, toPublicServiceJob } from './utils.js' export class ServiceStopHandler extends CommandHandler { @@ -36,12 +37,27 @@ export class ServiceStopHandler extends CommandHandler { status: { httpStatus: 503, error: 'Compute engines not configured' } } - // Find the job and the engine that owns it (by clusterHash — see helper) - const { job, engine } = await findServiceJobAndEngine( + // Find the job and the engine that owns it (by clusterHash — see helper). Scoped to + // the caller first, so a stranger cannot tell an existing service from a missing one. + let { job, engine } = await findServiceJobAndEngine( engines, task.serviceId, task.consumerAddress ) + // Node admins (ALLOWED_ADMINS / ALLOWED_ADMINS_LIST) may stop ANY service on this + // node — the operator has to be able to tear down a tenant's container without its + // key. Only consulted when the owner-scoped path did not already grant access, so the + // common owner call never pays for the access-list lookups. + let asAdmin = false + if (!job || job.owner.toLowerCase() !== task.consumerAddress.toLowerCase()) { + asAdmin = await isAllowedAdminAddress( + this.getOceanNode().getAdminAddresses(), + task.consumerAddress + ) + // Admin caller: redo the lookup unfiltered, since the job belongs to someone else. + if (asAdmin && !job) + ({ job, engine } = await findServiceJobAndEngine(engines, task.serviceId)) + } if (!job) return buildInvalidParametersResponse( buildInvalidRequestMessage('Service job not found: ' + task.serviceId) @@ -54,13 +70,21 @@ export class ServiceStopHandler extends CommandHandler { error: `No compute engine owns service ${task.serviceId} (cluster ${job.clusterHash}) — the node's compute configuration may have changed` } } - if (job.owner.toLowerCase() !== task.consumerAddress.toLowerCase()) + if (!asAdmin && job.owner.toLowerCase() !== task.consumerAddress.toLowerCase()) return { stream: null, status: { httpStatus: 401, error: 'Not the service owner' } } + if (asAdmin) + CORE_LOGGER.logMessage( + `Admin ${task.consumerAddress} is stopping service ${task.serviceId} owned by ${job.owner} (release=${task.release === true})`, + true + ) + try { const stopped = await engine.stopService( task.serviceId, - task.consumerAddress, + // The job's own owner, not the caller: an admin stop must still resolve the row + // the owner-scoped engine lookup expects. + job.owner, false, // onlyIfExpired task.release === true ) diff --git a/src/test/unit/service/serviceHandlers.test.ts b/src/test/unit/service/serviceHandlers.test.ts index fe18f2efd..fd90707ef 100644 --- a/src/test/unit/service/serviceHandlers.test.ts +++ b/src/test/unit/service/serviceHandlers.test.ts @@ -16,6 +16,9 @@ import { ServiceGetStreamableLogsHandler } from '../../../components/core/servic // Checksummed (EIP-55): commands are canonicalized on ingress, so this is the form handlers // see and forward, whatever casing the caller sent (see the lowercase-address test below). const OWNER = '0x0000000000000000000000000000000000000aBc' +// A node admin (ALLOWED_ADMINS) and an unrelated caller — neither owns the fake job. +const ADMIN = '0x0000000000000000000000000000000000000AdE' +const STRANGER = '0x0000000000000000000000000000000000000fFf' function makeJob(overrides: Partial = {}): ServiceJob { return { @@ -66,6 +69,8 @@ interface FakeOpts { cost?: number | null envId?: string streamableLogs?: Readable | null + // node admins (ALLOWED_ADMINS); empty unless a test grants one + admins?: string[] } function buildFakes(opts: FakeOpts = {}) { @@ -206,6 +211,11 @@ function buildFakes(opts: FakeOpts = {}) { validateAuthenticationOrToken: ({ address }: any) => Promise.resolve({ valid: true, address }) }), + // No ALLOWED_ADMINS by default — tests that exercise the admin path override this. + getAdminAddresses: () => ({ + addresses: opts.admins ?? [], + accessLists: undefined as any + }), getPersistentStorage: () => persistentStorage } @@ -338,6 +348,89 @@ describe('Service handlers', () => { expect(engine.stopService.calledOnce).to.equal(true) expect(engine.db.getServiceJob.firstCall.args[1]).to.equal(OWNER) }) + + it('200 when a node admin stops a service owned by someone else', async () => { + const { node, engine } = buildFakes({ + serviceJobInDb: makeJob(), + admins: [ADMIN] + }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: ADMIN + } as any) + expect(res.status.httpStatus).to.equal(200) + expect(engine.stopService.calledOnce).to.equal(true) + // the lookup is still owner-scoped first — the admin check is the fallback + expect(engine.db.getServiceJob.firstCall.args[1]).to.equal(ADMIN) + // the engine is asked for the JOB's owner, not the admin + expect(engine.stopService.firstCall.args[1]).to.equal(OWNER) + }) + + it('admin lookup falls back to an unfiltered query when the owner-scoped one misses', async () => { + const { node, engine } = buildFakes({ serviceJobInDb: makeJob(), admins: [ADMIN] }) + // The real DB filters on `owner`, so an admin's owner-scoped lookup finds nothing. + const job = makeJob() + engine.db.getServiceJob = sinon + .stub() + .callsFake((_serviceId: string, owner?: string) => + Promise.resolve( + !owner || owner.toLowerCase() === OWNER.toLowerCase() ? [job] : [] + ) + ) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: ADMIN + } as any) + expect(res.status.httpStatus).to.equal(200) + expect(engine.db.getServiceJob.callCount).to.equal(2) + expect(engine.db.getServiceJob.lastCall.args[1]).to.equal(undefined) + expect(engine.stopService.firstCall.args[1]).to.equal(OWNER) + }) + + it('admin stop forwards release', async () => { + const { node, engine } = buildFakes({ serviceJobInDb: makeJob(), admins: [ADMIN] }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: ADMIN, + release: true + } as any) + expect(res.status.httpStatus).to.equal(200) + expect(engine.stopService.firstCall.args[3]).to.equal(true) + }) + + it('admin matching is case-insensitive on the allowed-admins list', async () => { + const { node } = buildFakes({ + serviceJobInDb: makeJob(), + admins: [ADMIN.toLowerCase()] + }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: ADMIN + } as any) + expect(res.status.httpStatus).to.equal(200) + }) + + it("401 when a non-admin stranger stops someone else's service", async () => { + const { node, engine } = buildFakes({ + serviceJobInDb: makeJob(), + admins: [ADMIN] + }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: STRANGER + } as any) + expect(res.status.httpStatus).to.equal(401) + expect(engine.stopService.called).to.equal(false) + }) + + it('400 for an admin when the service does not exist at all', async () => { + const { node } = buildFakes({ serviceJobInDb: null, admins: [ADMIN] }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: ADMIN + } as any) + expect(res.status.httpStatus).to.equal(400) + }) }) describe('ServiceRestartHandler', () => { From d7355ffa2a597340c6d4ec0b3318ddabed725566 Mon Sep 17 00:00:00 2001 From: Denis <61563365+dnsi0@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:57:32 +0300 Subject: [PATCH 2/3] fix admin check --- src/components/core/admin/adminHandler.ts | 26 ++++----- src/components/core/service/stopService.ts | 5 +- src/test/unit/service/serviceHandlers.test.ts | 55 +++++++++++++++++-- src/utils/accessList.ts | 3 +- 4 files changed, 64 insertions(+), 25 deletions(-) diff --git a/src/components/core/admin/adminHandler.ts b/src/components/core/admin/adminHandler.ts index feae00a39..2dd5b9a5f 100644 --- a/src/components/core/admin/adminHandler.ts +++ b/src/components/core/admin/adminHandler.ts @@ -14,6 +14,8 @@ import { ReadableString } from '../../P2P/handleProtocolCommands.js' import { CommonValidation } from '../../../utils/validators.js' import { CORE_LOGGER } from '../../../utils/logging/common.js' import { normalizeCommandAddresses } from '../../../utils/evmAddress.js' +import { checkAddressOnAccessList } from '../../../utils/accessList.js' +import type { OceanNode } from '../../../OceanNode.js' // Membership test for the node's admin set: the ALLOWED_ADMINS address list first, then // each configured admin access list (ALLOWED_ADMINS_LIST), per chain. Says nothing about @@ -21,9 +23,10 @@ import { normalizeCommandAddresses } from '../../../utils/evmAddress.js' // auth token). Exported because handlers outside the admin family (SERVICE_STOP) also // grant the node operator a privileged path and must not re-implement these checks. export async function isAllowedAdminAddress( - allowedAdmins: { addresses: string[]; accessLists: any } | null | undefined, + oceanNode: OceanNode, address: string ): Promise { + const allowedAdmins = oceanNode.getAdminAddresses() if (!allowedAdmins || !address) { return false } @@ -36,21 +39,12 @@ export async function isAllowedAdminAddress( if (isListedAddress) { return true } + // The access-list balanceOf needs the chain's RPC signer. checkSingleCredential cannot + // be used here: it takes ONE contract address and the signer from its caller, while + // accessLists maps chainId → address[] — passing that array with a null signer made + // every ALLOWED_ADMINS_LIST member fail ("invalid value for Contract target"). if (accessLists) { - for (const chainId of Object.keys(accessLists)) { - const isOnAccessList = await checkSingleCredential( - { - type: CREDENTIALS_TYPES.ACCESS_LIST, - chainId: parseInt(chainId), - accessList: accessLists[chainId] - }, - address, - null - ) - if (isOnAccessList) { - return true - } - } + return await checkAddressOnAccessList(address, [accessLists], oceanNode) } return false } @@ -109,7 +103,7 @@ export abstract class AdminCommandHandler } } try { - if (await isAllowedAdminAddress(oceanNode.getAdminAddresses(), address)) { + if (await isAllowedAdminAddress(oceanNode, address)) { return { valid: true, error: '' } } diff --git a/src/components/core/service/stopService.ts b/src/components/core/service/stopService.ts index ef391218f..bf8a64f38 100644 --- a/src/components/core/service/stopService.ts +++ b/src/components/core/service/stopService.ts @@ -50,10 +50,7 @@ export class ServiceStopHandler extends CommandHandler { // common owner call never pays for the access-list lookups. let asAdmin = false if (!job || job.owner.toLowerCase() !== task.consumerAddress.toLowerCase()) { - asAdmin = await isAllowedAdminAddress( - this.getOceanNode().getAdminAddresses(), - task.consumerAddress - ) + asAdmin = await isAllowedAdminAddress(this.getOceanNode(), task.consumerAddress) // Admin caller: redo the lookup unfiltered, since the job belongs to someone else. if (asAdmin && !job) ({ job, engine } = await findServiceJobAndEngine(engines, task.serviceId)) diff --git a/src/test/unit/service/serviceHandlers.test.ts b/src/test/unit/service/serviceHandlers.test.ts index fd90707ef..804d04e3f 100644 --- a/src/test/unit/service/serviceHandlers.test.ts +++ b/src/test/unit/service/serviceHandlers.test.ts @@ -1,6 +1,7 @@ import { assert, expect } from 'chai' import { Readable } from 'stream' import sinon from 'sinon' +import { AbiCoder } from 'ethers' import { streamToObject } from '../../../utils/util.js' import { PROTOCOL_COMMANDS } from '../../../utils/constants.js' import { ServiceStatusNumber, ServiceJob } from '../../../@types/C2D/ServiceOnDemand.js' @@ -17,8 +18,8 @@ import { ServiceGetStreamableLogsHandler } from '../../../components/core/servic // see and forward, whatever casing the caller sent (see the lowercase-address test below). const OWNER = '0x0000000000000000000000000000000000000aBc' // A node admin (ALLOWED_ADMINS) and an unrelated caller — neither owns the fake job. -const ADMIN = '0x0000000000000000000000000000000000000AdE' -const STRANGER = '0x0000000000000000000000000000000000000fFf' +const ADMIN = '0x0000000000000000000000000000000000000adE' +const STRANGER = '0x0000000000000000000000000000000000000FfF' function makeJob(overrides: Partial = {}): ServiceJob { return { @@ -71,6 +72,10 @@ interface FakeOpts { streamableLogs?: Readable | null // node admins (ALLOWED_ADMINS); empty unless a test grants one admins?: string[] + // admin access lists (ALLOWED_ADMINS_LIST), chainId → contract addresses + adminAccessLists?: Record + // balanceOf answered by every access-list contract on the fake chain + accessListBalance?: number } function buildFakes(opts: FakeOpts = {}) { @@ -201,7 +206,21 @@ function buildFakes(opts: FakeOpts = {}) { getRequestMap: () => new Map(), getConfig: (): any => ({ rateLimit: undefined as number | undefined, - serviceTemplatesPath: undefined as string | undefined + serviceTemplatesPath: undefined as string | undefined, + supportedNetworks: { '8453': { chainId: 8453 } } + }), + // RPC signer for access-list balanceOf: a bare runner whose eth_call returns the balance + getBlockchain: () => ({ + getSigner: () => + Promise.resolve({ + call: () => + Promise.resolve( + AbiCoder.defaultAbiCoder().encode( + ['uint256'], + [opts.accessListBalance ?? 0] + ) + ) + }) }), getC2DEngines: () => engines, getKeyManager: () => ({ @@ -214,7 +233,7 @@ function buildFakes(opts: FakeOpts = {}) { // No ALLOWED_ADMINS by default — tests that exercise the admin path override this. getAdminAddresses: () => ({ addresses: opts.admins ?? [], - accessLists: undefined as any + accessLists: opts.adminAccessLists as any }), getPersistentStorage: () => persistentStorage } @@ -410,6 +429,34 @@ describe('Service handlers', () => { expect(res.status.httpStatus).to.equal(200) }) + it('200 when the admin is a member of an ALLOWED_ADMINS_LIST access list', async () => { + const { node, engine } = buildFakes({ + serviceJobInDb: makeJob(), + adminAccessLists: { '8453': ['0x00000000000000000000000000000000000000A1'] }, + accessListBalance: 1 + }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: ADMIN + } as any) + expect(res.status.httpStatus).to.equal(200) + expect(engine.stopService.firstCall.args[1]).to.equal(OWNER) + }) + + it('401 when the caller holds no token on the admin access list', async () => { + const { node, engine } = buildFakes({ + serviceJobInDb: makeJob(), + adminAccessLists: { '8453': ['0x00000000000000000000000000000000000000A1'] }, + accessListBalance: 0 + }) + const res = await new ServiceStopHandler(node).handle({ + ...baseTask, + consumerAddress: STRANGER + } as any) + expect(res.status.httpStatus).to.equal(401) + expect(engine.stopService.called).to.equal(false) + }) + it("401 when a non-admin stranger stops someone else's service", async () => { const { node, engine } = buildFakes({ serviceJobInDb: makeJob(), diff --git a/src/utils/accessList.ts b/src/utils/accessList.ts index 21d7671f3..e4d7e4ffb 100644 --- a/src/utils/accessList.ts +++ b/src/utils/accessList.ts @@ -56,8 +56,9 @@ export async function checkAddressOnAccessList( for (const accessListMap of access) { if (!accessListMap) continue for (const chain of Object.keys(accessListMap)) { - const { chainId } = supportedNetworks[chain] try { + // inside the try: a list on a chain missing from supportedNetworks must skip, not throw + const { chainId } = supportedNetworks[chain] const blockchain = oceanNode.getBlockchain(chainId) if (!blockchain) { CORE_LOGGER.logMessage( From 65609cef171887e99e31c5498b81f9f07e6ed29d Mon Sep 17 00:00:00 2001 From: Denis <61563365+dnsi0@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:05:53 +0300 Subject: [PATCH 3/3] fix admin check --- package-lock.json | 18 ++- src/components/core/admin/adminHandler.ts | 119 +++++++++++------- src/test/unit/service/serviceHandlers.test.ts | 3 +- src/utils/accessList.ts | 3 +- 4 files changed, 91 insertions(+), 52 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9c14eceb9..57fa61230 100644 --- a/package-lock.json +++ b/package-lock.json @@ -189,6 +189,7 @@ "resolved": "https://registry.npmjs.org/@aws-sdk/client-s3/-/client-s3-3.1115.0.tgz", "integrity": "sha512-oeniaXZCRrKMaffnyjOSxp1xJNsuiku2SxyzVI1Bi1Gycpck/dRTVsloSa5E+nBKz1c5y5eMfbK4GAhGUCCC2Q==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@aws-sdk/checksums": "^3.1000.28", "@aws-sdk/core": "^3.977.8", @@ -527,6 +528,7 @@ "integrity": "sha512-RgHBCvtjbOK2gXSNBNIkNoEc9qoVEtau3hj8gEqKQuL3HZAibKarWFEI3Lfm6EYKkLalOh8eSrj9b+ch9H/VBA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.7", "@babel/generator": "^7.29.7", @@ -3003,6 +3005,7 @@ "integrity": "sha512-DcB0M3KFgr9ECI328lhBMVsyFT2DnmNucSBTqEN3exyNKUzkkpUSCHmTRcunF41Eou2TIQKW4seewri8ON9bSA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.4", @@ -3207,6 +3210,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.1.tgz", "integrity": "sha512-gLyJlPHPZYdAk1JENA9LeHejZe1Ti77/pTeFm/nMXmQH/HFZlcS/O2XJB+L8fkbrNSqhdtlvjBVjxwUYanNH5Q==", "license": "Apache-2.0", + "peer": true, "engines": { "node": ">=8.0.0" } @@ -4150,6 +4154,7 @@ "resolved": "https://registry.npmjs.org/@rdfjs/types/-/types-2.0.1.tgz", "integrity": "sha512-uyAzpugX7KekAXAHq26m3JlUIZJOC0uSBhpnefGV5i15bevDyyejoB7I+9MKeUrzXD8OOUI3+4FeV1wwQr5ihA==", "license": "MIT", + "peer": true, "dependencies": { "@types/node": "*" } @@ -4736,6 +4741,7 @@ "integrity": "sha512-fUBfTuuEulWqX6V8+O3PtScV01tzYYRUDTAirHFKoRAt7nOzoGiPt0M/bB47wWNy0coOOcgEwAMUtBpykMxl6w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.67.0", "@typescript-eslint/types": "8.67.0", @@ -5077,6 +5083,7 @@ "integrity": "sha512-lGq+9yr1/GuAWaVYIHRjvvySG5/4VfKIvC8EWxStPdcDh/Ka7FG3twP6v4d5BkravUilhIAsG4Qj83t02LWUPQ==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -5376,6 +5383,7 @@ "resolved": "https://registry.npmjs.org/bare-events/-/bare-events-2.9.1.tgz", "integrity": "sha512-Z0oHEHAFDZkffN8Qc39zNZjQlMDkPJRyyyZieU1VH7u8c5S+qHZ2S8ixdKIAxEjfHO7FJxXmJWgteOghVanIsg==", "license": "Apache-2.0", + "peer": true, "peerDependencies": { "bare-abort-controller": "*" }, @@ -5636,6 +5644,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.11.12", "caniuse-lite": "^1.0.30001809", @@ -6680,6 +6689,7 @@ "resolved": "https://registry.npmjs.org/@noble/ciphers/-/ciphers-1.3.0.tgz", "integrity": "sha512-2I0gnIVPtfnMw9ee9h1dJG7tp81+8Ob3OJb3Mv37rx5L40/b0i7djjCVvGOVqc9AEIQyvyu1i6ypKdFw8R8gQw==", "license": "MIT", + "peer": true, "engines": { "node": "^14.21.3 || >=16" }, @@ -6878,6 +6888,7 @@ "integrity": "sha512-wqA7W2jbsC/BnV9Iv1UZpKVFkO1AdNoSmYW8NWG4HNOBbkAMvIqDZ27pI2f07dqn583NcIC44ckjAcOXDL1QbQ==", "dev": true, "license": "MIT", + "peer": true, "workspaces": [ "packages/*" ], @@ -6937,6 +6948,7 @@ "integrity": "sha512-82GZUjRS0p/jganf6q1rEO25VSoHH0hKPCTrgillPjdI/3bgBhAE1QzHrHTizjpRvy6pGAvKjDJtk2pF9NDq8w==", "dev": true, "license": "MIT", + "peer": true, "bin": { "eslint-config-prettier": "bin/cli.js" }, @@ -10924,6 +10936,7 @@ "integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -11041,6 +11054,7 @@ "integrity": "sha512-OpN0zzVdiaiAhxpuuj5efpIS4sY9j7bY6uR5mnj5yPzGkdkjNKSJeUThPb60Jw29QuAZgA4o+/iB49kFiaBX6g==", "dev": true, "license": "MIT", + "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -11321,7 +11335,8 @@ "resolved": "https://registry.npmjs.org/quickjs-wasi/-/quickjs-wasi-2.2.0.tgz", "integrity": "sha512-zQxXmQMrEoD3S+jQdYsloq4qAuaxKFHZj6hHqOYGwB2iQZH+q9e/lf5zQPXCKOk0WJuAjzRFbO4KwHIp2D05Iw==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/race-event": { "version": "1.6.1", @@ -12928,6 +12943,7 @@ "integrity": "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/src/components/core/admin/adminHandler.ts b/src/components/core/admin/adminHandler.ts index 125c8f10b..d97d56bef 100644 --- a/src/components/core/admin/adminHandler.ts +++ b/src/components/core/admin/adminHandler.ts @@ -18,6 +18,78 @@ import { CommonValidation } from '../../../utils/validators.js' import { CORE_LOGGER } from '../../../utils/logging/common.js' import { normalizeCommandAddresses } from '../../../utils/evmAddress.js' import { isAddress } from 'ethers' +import type { OceanNode } from '../../../OceanNode.js' + +// Membership test for the node's admin set: the ALLOWED_ADMINS address list first, then +// each configured admin access list (ALLOWED_ADMINS_LIST), per chain. Says nothing about +// authentication — the caller must have already proven it owns `address` (signature or +// auth token). Exported because handlers outside the admin family (SERVICE_STOP) also +// grant the node operator a privileged path and must not re-implement these checks. +export async function isAllowedAdminAddress( + oceanNode: OceanNode, + address: string +): Promise { + const allowedAdmins = oceanNode.getAdminAddresses() + if (!allowedAdmins || !address) { + return false + } + const { addresses, accessLists } = allowedAdmins + const isListedAddress = await checkSingleCredential( + { type: CREDENTIALS_TYPES.ADDRESS, values: addresses }, + address, + null + ) + if (isListedAddress) { + return true + } + if (accessLists) { + for (const chainId of Object.keys(accessLists)) { + // Need an on-chain signer/provider to call balanceOf on the access list + // contract. getBlockchain() returns null when that chain has no RPC configured. + const blockchain = oceanNode.getBlockchain(parseInt(chainId)) + if (!blockchain) { + CORE_LOGGER.error( + `Cannot check admin access list for chain ${chainId}: no RPC configured for that chain. Skipping.` + ) + continue + } + // Fail closed on misconfiguration: an empty or malformed contract address + // would make checkAddressOnAccessListWithSigner return `true` (it treats a + // falsy address as "no access list"), silently authorizing ANY authenticated + // caller as admin. Only keep well-formed contract addresses. + const validContracts = accessLists[chainId].filter((addr: string) => + isAddress(addr) + ) + if (validContracts.length === 0) { + CORE_LOGGER.error( + `No valid access list contract address configured for admin check on chain ${chainId}. Skipping.` + ) + continue + } + try { + const signer = await blockchain.getSigner() + // Pass only the validated contracts for this chain; checkCredentialOnAccessList + // iterates the array and checks each one with an on-chain balanceOf. + const isOnAccessList = await checkCredentialOnAccessList( + { [chainId]: validContracts }, + chainId, + address, + signer + ) + if (isOnAccessList) { + return true + } + } catch (error) { + // Isolate per-chain failures (RPC rate limit / downtime) so one bad + // chain does not abort the whole loop and deny an otherwise-valid admin. + CORE_LOGGER.error( + `Error checking admin access list for chain ${chainId}: ${error}` + ) + } + } + } + return false +} export abstract class AdminCommandHandler extends BaseHandler @@ -76,53 +148,6 @@ export abstract class AdminCommandHandler if (await isAllowedAdminAddress(oceanNode, address)) { return { valid: true, error: '' } } - if (accessLists) { - for (const chainId of Object.keys(accessLists)) { - // Need an on-chain signer/provider to call balanceOf on the access list - // contract. getBlockchain() returns null when that chain has no RPC configured. - const blockchain = oceanNode.getBlockchain(parseInt(chainId)) - if (!blockchain) { - CORE_LOGGER.error( - `Cannot check admin access list for chain ${chainId}: no RPC configured for that chain. Skipping.` - ) - continue - } - // Fail closed on misconfiguration: an empty or malformed contract address - // would make checkAddressOnAccessListWithSigner return `true` (it treats a - // falsy address as "no access list"), silently authorizing ANY authenticated - // caller as admin. Only keep well-formed contract addresses. - const validContracts = accessLists[chainId].filter((addr: string) => - isAddress(addr) - ) - if (validContracts.length === 0) { - CORE_LOGGER.error( - `No valid access list contract address configured for admin check on chain ${chainId}. Skipping.` - ) - continue - } - try { - const signer = await blockchain.getSigner() - // Pass only the validated contracts for this chain; checkCredentialOnAccessList - // iterates the array and checks each one with an on-chain balanceOf. - allowed = await checkCredentialOnAccessList( - { [chainId]: validContracts }, - chainId, - address, - signer - ) - } catch (error) { - // Isolate per-chain failures (RPC rate limit / downtime) so one bad - // chain does not abort the whole loop and deny an otherwise-valid admin. - CORE_LOGGER.error( - `Error checking admin access list for chain ${chainId}: ${error}` - ) - continue - } - if (allowed) { - return { valid: true, error: '' } - } - } - } const errorMsg = `The address which signed the message is not on the allowed admins list. Therefore signature ${signature} is rejected` CORE_LOGGER.logMessage(errorMsg) diff --git a/src/test/unit/service/serviceHandlers.test.ts b/src/test/unit/service/serviceHandlers.test.ts index 804d04e3f..440185516 100644 --- a/src/test/unit/service/serviceHandlers.test.ts +++ b/src/test/unit/service/serviceHandlers.test.ts @@ -206,8 +206,7 @@ function buildFakes(opts: FakeOpts = {}) { getRequestMap: () => new Map(), getConfig: (): any => ({ rateLimit: undefined as number | undefined, - serviceTemplatesPath: undefined as string | undefined, - supportedNetworks: { '8453': { chainId: 8453 } } + serviceTemplatesPath: undefined as string | undefined }), // RPC signer for access-list balanceOf: a bare runner whose eth_call returns the balance getBlockchain: () => ({ diff --git a/src/utils/accessList.ts b/src/utils/accessList.ts index e4d7e4ffb..21d7671f3 100644 --- a/src/utils/accessList.ts +++ b/src/utils/accessList.ts @@ -56,9 +56,8 @@ export async function checkAddressOnAccessList( for (const accessListMap of access) { if (!accessListMap) continue for (const chain of Object.keys(accessListMap)) { + const { chainId } = supportedNetworks[chain] try { - // inside the try: a list on a chain missing from supportedNetworks must skip, not throw - const { chainId } = supportedNetworks[chain] const blockchain = oceanNode.getBlockchain(chainId) if (!blockchain) { CORE_LOGGER.logMessage(