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] 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', () => {