diff --git a/packages/general/src/transaction/errors.ts b/packages/general/src/transaction/errors.ts index ad9c23dac..0e0450edd 100644 --- a/packages/general/src/transaction/errors.ts +++ b/packages/general/src/transaction/errors.ts @@ -42,6 +42,12 @@ export class FinalizationError extends MatterError {} export class TransactionDestroyedError extends MatterError {} /** - * Trhown if a {@link Transaction} cannot commit because state has mutated continuously for too many pre-commit events. + * Thrown if a {@link Transaction} cannot commit because state has mutated continuously for too many pre-commit cycles. + * + * "Pre-commit" is a commit event triggered by {@link Transaction} before stage 1 commit. During pre-commit listeners + * may mutate state. If state does change, the transaction re-runs pre-commit so all listeners see the same state. + * + * If state continues to mutate for too many of these cycles then the transaction will abort. This likely indicates a + * logic error that will result in an infinite loop. */ export class UnsettledStateError extends FinalizationError {} diff --git a/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts b/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts index a25292749..918952b4d 100644 --- a/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts +++ b/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts @@ -6,8 +6,8 @@ import { Behavior } from "#behavior/Behavior.js"; import { ClusterBehavior } from "#index.js"; +import { Schema } from "#model"; import { ClusterType } from "#types"; -import { Schema } from "../supervision/Schema.js"; /** * To save memory we cache behavior implementations specialized for specific clusters. This allows for efficient diff --git a/packages/node/src/behavior/context/server/OnlineContext.ts b/packages/node/src/behavior/context/server/OnlineContext.ts index 3bc390ad3..80afd7176 100644 --- a/packages/node/src/behavior/context/server/OnlineContext.ts +++ b/packages/node/src/behavior/context/server/OnlineContext.ts @@ -27,6 +27,7 @@ export function OnlineContext(options: OnlineContext.Options) { let fabric: FabricIndex | undefined; let subject: SubjectId; let nodeProtocol: NodeProtocol | undefined; + let accessLevelCache: Map | undefined; const { exchange } = options; const session = exchange?.session; @@ -167,8 +168,9 @@ export function OnlineContext(options: OnlineContext.Options) { } // We already checked access levels in this transaction, so reuse it - if (location.accessLevels !== undefined) { - return location.accessLevels.includes(desiredAccessLevel) + const cachedAccessLevels = accessLevelCache?.get(location); + if (cachedAccessLevels !== undefined) { + return cachedAccessLevels.includes(desiredAccessLevel) ? AccessControl.Authority.Granted : AccessControl.Authority.Unauthorized; } @@ -182,7 +184,12 @@ export function OnlineContext(options: OnlineContext.Options) { throw new InternalError("AccessControlServer should already be initialized."); } const accessLevels = accessControl.accessLevelsFor(context, location, aclEndpointContextFor(location)); - location.accessLevels = accessLevels; + + if (accessLevelCache === undefined) { + accessLevelCache = new Map(); + } + accessLevelCache.set(location, accessLevels); + return accessLevels.includes(desiredAccessLevel) ? AccessControl.Authority.Granted : AccessControl.Authority.Unauthorized; diff --git a/packages/node/src/behavior/state/managed/Datasource.ts b/packages/node/src/behavior/state/managed/Datasource.ts index 4bfa2165f..f0d9b9489 100644 --- a/packages/node/src/behavior/state/managed/Datasource.ts +++ b/packages/node/src/behavior/state/managed/Datasource.ts @@ -253,6 +253,9 @@ function configure(options: Datasource.Options): Internals { values[key] = initialValues[key]; } + // Location affects security so make it immutable + Object.freeze(options.location); + return { ...options, version: Crypto.getRandomUInt32(), diff --git a/packages/node/src/node/server/TransactionalInteractionServer.ts b/packages/node/src/node/server/TransactionalInteractionServer.ts index 90a9e3725..6c0106158 100644 --- a/packages/node/src/node/server/TransactionalInteractionServer.ts +++ b/packages/node/src/node/server/TransactionalInteractionServer.ts @@ -36,7 +36,7 @@ import { WriteRequest, WriteResponse, } from "#protocol"; -import { EndpointNumber, StatusCode, StatusResponseError, TlvEventFilter, TypeFromSchema } from "#types"; +import { StatusCode, StatusResponseError, TlvEventFilter, TypeFromSchema } from "#types"; import { AccessControlServer } from "../../behaviors/access-control/AccessControlServer.js"; import { ServerNode } from "../ServerNode.js"; @@ -368,7 +368,7 @@ export class TransactionalInteractionServer extends InteractionServer { const invokeCommand = (context: ActionContext) => { if ( context.authorityAt(command.invokeAcl, { - endpoint: EndpointNumber(1), + endpoint: endpoint.number, cluster: path.clusterId, } as AccessControl.Location) !== AccessControl.Authority.Granted ) { diff --git a/packages/protocol/src/action/request/Invoke.ts b/packages/protocol/src/action/request/Invoke.ts index 4636efffd..03ebbe8d8 100644 --- a/packages/protocol/src/action/request/Invoke.ts +++ b/packages/protocol/src/action/request/Invoke.ts @@ -17,17 +17,17 @@ export interface Invoke extends InvokeRequest { * Request invocation of one or more commands. */ export function Invoke(definition: Invoke.Definition): Invoke { - let { interactionModelRevision, suppressResponse, timed: timedRequest } = definition; - const { commands } = definition; + const { + commands, + interactionModelRevision = FALLBACK_INTERACTIONMODEL_REVISION, + suppressResponse = false, + timed: timedRequest = false, + } = definition; if (!commands?.length) { throw new MalformedRequestError(`Invocation requires at least one command`); } - interactionModelRevision ??= FALLBACK_INTERACTIONMODEL_REVISION; - suppressResponse ??= false; - timedRequest ??= false; - return { invokeRequests: commands, interactionModelRevision, diff --git a/packages/protocol/src/action/request/MalformedRequestError.ts b/packages/protocol/src/action/request/MalformedRequestError.ts index bda22d67a..9e657dd06 100644 --- a/packages/protocol/src/action/request/MalformedRequestError.ts +++ b/packages/protocol/src/action/request/MalformedRequestError.ts @@ -4,9 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { MatterError } from "#general"; +import { ImplementationError } from "#general"; /** * Thrown when an action request does not adhere to the Matter specification. + * + * This is a client-side error that throws while formulating a request. */ -export class MalformedRequestError extends MatterError {} +export class MalformedRequestError extends ImplementationError {} diff --git a/packages/protocol/src/action/request/Read.ts b/packages/protocol/src/action/request/Read.ts index 79f37bd1a..5aa56c266 100644 --- a/packages/protocol/src/action/request/Read.ts +++ b/packages/protocol/src/action/request/Read.ts @@ -102,7 +102,7 @@ export function Read(optionsOrSelector: Read.Options | Read.Selector, ...selecto const { endpoint } = selector; // Install data version filter if the endpoint reports it has complete version information - if (typeof endpoint === "object" && typeof endpoint.versions === "object" && typeof cluster === "object") { + if (typeof endpoint === "object" && endpoint?.versions && typeof cluster === "object" && cluster !== null) { const version = endpoint.versions?.[camelize(cluster.name)]; if (version !== undefined) { const filter = { diff --git a/packages/protocol/src/action/server/AccessControl.ts b/packages/protocol/src/action/server/AccessControl.ts index 408a23674..b8f83f147 100644 --- a/packages/protocol/src/action/server/AccessControl.ts +++ b/packages/protocol/src/action/server/AccessControl.ts @@ -117,12 +117,6 @@ export namespace AccessControl { * enforcement. */ owningFabric?: FabricIndex; - - /** - * The access levels already retrieved for this location. With this subtree elements can access the same - * access levels without re-evaluating. - */ - accessLevels?: AccessLevel[]; } /**