Node ACLs are authored and deployed but never enforced — wire IPermissionEvaluator or retire the surface #520

Closed
opened 2026-07-27 18:35:58 -04:00 by dohertj2 · 8 comments
Owner

Split out of the deferment.md §3.1 audit. Decision taken 2026-07-27: make the non-enforcement explicit now (docs + UI warnings — done), and track the real wire-up here.

What is true today

  • IPermissionEvaluator, TriePermissionEvaluator, PermissionTrieCache, PermissionTrieBuilder live in src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/ with 30 passing unit tests.
  • IPermissionEvaluator.Authorize has zero production call sites. Nothing registers it in DI.
  • ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj does not reference ZB.MOM.WW.OtOpcUa.Core at all — the evaluator is not even on the node manager's compile-time reference graph.
  • Operators can author NodeAcl rows (/clusters/{id}/acls), and ConfigComposer snapshots every row into every deployment artifact — so an ACL edit shifts the artifact RevisionHash and ships fleet-wide.
  • DeploymentArtifact never deserializes them. The bytes arrive and are dropped.

Net: an operator can author a grant, deploy it green, and have it change nothing.

Actual enforcement is two server-wide role checks — WriteOperate (Value writes, either realm) and AlarmAck (all five Part 9 alarm methods). Read, Browse, HistoryRead and CreateMonitoredItems have no authorization check at all, so any admitted session including Anonymous reads the whole address space.

Four blockers — none of them is "call the evaluator"

  1. Raw-realm nodes have no representable scope. NodeHierarchyKind has exactly one member, Equipment. Raw NodeIds are Folder/Driver/Device/TagGroup/Tag RawPaths, which NodeScope cannot express — yet the write gate is attached to raw tags. Roughly half the writable address space has no scope to evaluate. This is a design decision, not wiring.
  2. The session carries roles, not LDAP groups. The trie matches NodeAcl.LdapGroup, but OpcUaUserAuthResult carries only mapped role strings; the group list is discarded inside LdapOpcUaUserAuthenticator.
  3. PermissionTrieBuilder's scopePaths argument has no producer anywhere in the tree. Building without it drops every sub-cluster grant into the root-child fallback — the hazard the builder's own comments warn about. Note every existing evaluator unit test runs in the no-scopePaths "deterministic test mode", so the production hierarchy path is effectively untested.
  4. No session lifecycle exists. UserAuthorizationState has the freshness/staleness predicates but nothing constructs one, stamps AuthGenerationId, or drives a refresh.

Plus: the node side never parses ACLs out of the artifact, and OtOpcUaNodeManager has no cluster identity (NodeScope.ClusterId is required and the evaluator fails closed on mismatch).

Smaller inconsistencies inside the subsystem

  • NodeAclScopeKind.FolderSegment is offered in the authoring dropdown but the trie walker has no level to match it at.
  • NodePermissions.AlarmRead has no OpcUaOperation mapping — grantable but unreachable.
  • NodeAcl.ScopeId has no FK and no existence check, so scope ids can dangle silently.
  • The SystemPlatform hierarchy kind that acl-design.md describes was retired with v3.

Scope of a fix

Write has a real chokepoint already (OnEquipmentTagWrite) — it runs under the node-manager Lock, so the check must stay non-blocking; the evaluator is pure, so that fits. Read has no seam at all — it needs either a Read override or a per-variable OnSimpleReadValue handler on the hot publish path.

Already done

docs/security.md, docs/ReadWriteOperations.md, docs/v2/acl-design.md and docs/v2/v2-release-readiness.md corrected; warning banners added to ClusterAcls.razor and AclEdit.razor. v2-release-readiness.md had marked this "CLOSED 2026-04-19, PR #94" with a detailed description of Browse/CreateMonitoredItems/Call gating — none of those types exist in src/.

Split out of the `deferment.md` §3.1 audit. **Decision taken 2026-07-27: make the non-enforcement explicit now (docs + UI warnings — done), and track the real wire-up here.** ## What is true today - `IPermissionEvaluator`, `TriePermissionEvaluator`, `PermissionTrieCache`, `PermissionTrieBuilder` live in `src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/` with 30 passing unit tests. - `IPermissionEvaluator.Authorize` has **zero production call sites**. Nothing registers it in DI. - `ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj` does not reference `ZB.MOM.WW.OtOpcUa.Core` at all — the evaluator is not even on the node manager's compile-time reference graph. - Operators **can** author `NodeAcl` rows (`/clusters/{id}/acls`), and `ConfigComposer` snapshots every row into every deployment artifact — so an ACL edit shifts the artifact `RevisionHash` and ships fleet-wide. - `DeploymentArtifact` never deserializes them. The bytes arrive and are dropped. Net: an operator can author a grant, deploy it green, and have it change nothing. Actual enforcement is two server-wide role checks — `WriteOperate` (Value writes, either realm) and `AlarmAck` (all five Part 9 alarm methods). **Read, Browse, HistoryRead and CreateMonitoredItems have no authorization check at all**, so any admitted session including Anonymous reads the whole address space. ## Four blockers — none of them is "call the evaluator" 1. **Raw-realm nodes have no representable scope.** `NodeHierarchyKind` has exactly one member, `Equipment`. Raw NodeIds are `Folder/Driver/Device/TagGroup/Tag` RawPaths, which `NodeScope` cannot express — yet the write gate is attached to raw tags. Roughly half the writable address space has no scope to evaluate. **This is a design decision, not wiring.** 2. **The session carries roles, not LDAP groups.** The trie matches `NodeAcl.LdapGroup`, but `OpcUaUserAuthResult` carries only mapped role strings; the group list is discarded inside `LdapOpcUaUserAuthenticator`. 3. **`PermissionTrieBuilder`'s `scopePaths` argument has no producer** anywhere in the tree. Building without it drops every sub-cluster grant into the root-child fallback — the hazard the builder's own comments warn about. Note every existing evaluator unit test runs in the no-`scopePaths` "deterministic test mode", so the production hierarchy path is effectively untested. 4. **No session lifecycle exists.** `UserAuthorizationState` has the freshness/staleness predicates but nothing constructs one, stamps `AuthGenerationId`, or drives a refresh. Plus: the node side never parses ACLs out of the artifact, and `OtOpcUaNodeManager` has no cluster identity (`NodeScope.ClusterId` is `required` and the evaluator fails closed on mismatch). ## Smaller inconsistencies inside the subsystem - `NodeAclScopeKind.FolderSegment` is offered in the authoring dropdown but the trie walker has no level to match it at. - `NodePermissions.AlarmRead` has no `OpcUaOperation` mapping — grantable but unreachable. - `NodeAcl.ScopeId` has no FK and no existence check, so scope ids can dangle silently. - The `SystemPlatform` hierarchy kind that `acl-design.md` describes was retired with v3. ## Scope of a fix Write has a real chokepoint already (`OnEquipmentTagWrite`) — it runs under the node-manager `Lock`, so the check must stay non-blocking; the evaluator is pure, so that fits. **Read has no seam at all** — it needs either a `Read` override or a per-variable `OnSimpleReadValue` handler on the hot publish path. ## Already done `docs/security.md`, `docs/ReadWriteOperations.md`, `docs/v2/acl-design.md` and `docs/v2/v2-release-readiness.md` corrected; warning banners added to `ClusterAcls.razor` and `AclEdit.razor`. `v2-release-readiness.md` had marked this "CLOSED 2026-04-19, PR #94" with a detailed description of Browse/CreateMonitoredItems/Call gating — none of those types exist in `src/`.
Author
Owner

Direction decided 2026-07-31, design written, stage 1 landed on feat/gitea-520-apikey-acl.

Decisions (owner)

# Decision
D1 The OPC UA data plane is machine-to-machine. Identity is an API key with a static username, not an LDAP user.
D2 Replace LDAP entirely on the data plane. LdapOpcUaUserAuthenticator and the OPC UA half of Security:Ldap go; AdminUI login keeps its own LDAP.
D3 Anonymous becomes a normal grantable principal behind an enable setting, rather than a special case.

D1 dissolves blocker #2 rather than solving it — the principal comes from the key, so there is no group list to plumb onto the session. D3 is the tidy one: there is no "anonymous bypass" branch in the evaluator to get wrong, because an ungranted anonymous session is denied by the same fail-closed path as an ungranted key.

Re-verification first

All six claims in the issue body still hold on master as of today. Two corrections to the issue's own survey, in both directions:

  • Better than stated: HistoryRead does have a seam — HistoryReadRawModified / Processed / AtTime / Events are already overridden, they just carry no check.
  • Confirmed as the real gap: Read / Browse / Subscribe genuinely have no override, and sit on the hot publish path, so decisions must be cached per (session, node) rather than evaluated per value.
  • Blocker #1 is smaller than it looks. NodeAclScopeKind already carries Namespace and FolderSegment. Adding NodeHierarchyKind.Raw over the RawPath segments makes raw-realm nodes representable and gives FolderSegment a level to match at for the first time — it does not need a NodeScope redesign.

Two things the issue did not record

  1. Enforcement ships behind Security:Acl:Enabled, default off — the same dark-switch discipline as MeshTransport / ConfigSource / Telemetry. Without it the first deploy carrying this code silently starts denying every client with no key, including ScadaBridge's Data Connection Layer, which consumes this address space from another repo.
  2. Key revocation will require a deploy. Phase 4 leaves driver-only nodes with no ConfigDb, so keys ride the deployment artifact exactly as NodeAcl rows do. Real sharp edge, documented rather than discovered.

Landed so far (stages 1a + 1b, behaviour-neutral)

  • NodeAcl.LdapGroup → Principal through the whole ACL model. LdapGroupRoleMapping is a different entity that also has an LdapGroup column and is untouched — it backs AdminUI login.
  • A trap worth recording: the column is also read by dbo.sp_ComputeGenerationDiff, created in V3Initial.StoredProcedures and never redefined. SQL Server does not validate a procedure body when a column is renamed under it, so the proc keeps reporting healthy and fails at CALL time with Invalid column name, silently breaking config-drift detection. Verified against the live dev DB that the deployed proc really does reference it. The migration re-emits the proc from one template shared by Up and Down.
  • ApiKey entity + ApiKeySecret (otua_<keyId>_<secret>, peppered HMAC-SHA256, fail-closed on an unset pepper), 18 new tests.
  • OpcUaServer.csproj now references Core — it did not before, so IPermissionEvaluator was literally uncallable from the node manager.

Core 247 / OpcUaServer 364 / Configuration 128 green. Migrations are deliberately not applied to the shared dev DB yet: the running containers still use the old model and ConfigComposer reads db.NodeAcls on every deploy, so applying before a rebuild would break the rig's deploy path.

Remaining stages

2 identity cutover · 3 raw scope + scopePaths producer · 4 artifact parsing + session lifecycle · 5 enforce write/alarms/HistoryRead · 6 enforce read/browse/subscribe · 7 AdminUI key management (and removing the two "not enforced" banners, which is the honest done-signal) · 8 docs + ScadaBridge key hand-off + live gate.

Design doc: docs/plans/2026-07-31-apikey-acl-enforcement-design.md.

**Direction decided 2026-07-31, design written, stage 1 landed on `feat/gitea-520-apikey-acl`.** ## Decisions (owner) | # | Decision | |---|---| | D1 | The OPC UA data plane is **machine-to-machine**. Identity is an **API key with a static username**, not an LDAP user. | | D2 | **Replace LDAP entirely** on the data plane. `LdapOpcUaUserAuthenticator` and the OPC UA half of `Security:Ldap` go; **AdminUI login keeps its own LDAP**. | | D3 | **Anonymous becomes a normal grantable principal** behind an enable setting, rather than a special case. | D1 **dissolves blocker #2** rather than solving it — the principal comes from the key, so there is no group list to plumb onto the session. D3 is the tidy one: there is no "anonymous bypass" branch in the evaluator to get wrong, because an ungranted anonymous session is denied by the same fail-closed path as an ungranted key. ## Re-verification first All six claims in the issue body still hold on `master` as of today. Two corrections to the issue's own survey, in both directions: - **Better than stated:** `HistoryRead` *does* have a seam — `HistoryReadRawModified` / `Processed` / `AtTime` / `Events` are already overridden, they just carry no check. - **Confirmed as the real gap:** Read / Browse / Subscribe genuinely have no override, and sit on the hot publish path, so decisions must be cached per (session, node) rather than evaluated per value. - **Blocker #1 is smaller than it looks.** `NodeAclScopeKind` already carries `Namespace` and `FolderSegment`. Adding `NodeHierarchyKind.Raw` over the RawPath segments makes raw-realm nodes representable *and* gives `FolderSegment` a level to match at for the first time — it does not need a `NodeScope` redesign. ## Two things the issue did not record 1. **Enforcement ships behind `Security:Acl:Enabled`, default off** — the same dark-switch discipline as `MeshTransport` / `ConfigSource` / `Telemetry`. Without it the first deploy carrying this code silently starts denying every client with no key, **including ScadaBridge's Data Connection Layer, which consumes this address space from another repo**. 2. **Key revocation will require a deploy.** Phase 4 leaves driver-only nodes with no ConfigDb, so keys ride the deployment artifact exactly as `NodeAcl` rows do. Real sharp edge, documented rather than discovered. ## Landed so far (stages 1a + 1b, behaviour-neutral) - `NodeAcl.LdapGroup` → `Principal` through the whole ACL model. `LdapGroupRoleMapping` is a *different* entity that also has an `LdapGroup` column and is untouched — it backs AdminUI login. - **A trap worth recording:** the column is also read by `dbo.sp_ComputeGenerationDiff`, created in `V3Initial.StoredProcedures` and never redefined. SQL Server does not validate a procedure body when a column is renamed under it, so the proc keeps reporting healthy and fails at CALL time with `Invalid column name`, silently breaking config-drift detection. Verified against the live dev DB that the deployed proc really does reference it. The migration re-emits the proc from one template shared by Up and Down. - `ApiKey` entity + `ApiKeySecret` (`otua_<keyId>_<secret>`, peppered HMAC-SHA256, fail-closed on an unset pepper), 18 new tests. - **`OpcUaServer.csproj` now references `Core`** — it did not before, so `IPermissionEvaluator` was literally uncallable from the node manager. Core 247 / OpcUaServer 364 / Configuration 128 green. Migrations are deliberately **not** applied to the shared dev DB yet: the running containers still use the old model and `ConfigComposer` reads `db.NodeAcls` on every deploy, so applying before a rebuild would break the rig's deploy path. ## Remaining stages 2 identity cutover · 3 raw scope + `scopePaths` producer · 4 artifact parsing + session lifecycle · 5 enforce write/alarms/HistoryRead · 6 enforce read/browse/subscribe · 7 AdminUI key management (and removing the two "not enforced" banners, which is the honest done-signal) · 8 docs + ScadaBridge key hand-off + live gate. Design doc: `docs/plans/2026-07-31-apikey-acl-enforcement-design.md`.
Author
Owner

Stage 3 done — 3a6b2a1d on feat/gitea-520-stage3-raw-scope (unmerged). Stages 1/2/4 merged earlier in fbe7403c.

Stage 3 makes the /raw device tree addressable by an ACL rule and builds the scopePaths producer. It was larger than planned: the design doc described a raw path as the fixed five-tuple Folder/Driver/Device/TagGroup/Tag, but folders and tag groups both self-nest, so NodeScope had to stop being fixed-arity entirely. Separately, level 1 of the walk gated on a NamespaceId that v3 has no entity for — it could only ever fail to match, which silently made every sub-cluster grant unreachable while cluster-scoped grants kept working. That level is now the realm, derived from the scope's kind.

Blockers 1, 3 and 4 from this issue are now closed. Blocker 2 was dissolved by the API-key model in stage 2.

⚠️ Node ACLs are still NOT enforced. Nothing constructs a NodeScope or calls the evaluator; only Cluster-scoped grants confer anything, via ApiKeyInterimRoleMapper. That is stages 5 (write / alarms / HistoryRead) and 6 (read / browse / subscribe). This stage makes the model able to express what those will enforce.

Four defects found by review while doing it, all fixed: #528 #529 #530 #531. Two deferred: #532 #533 — #533 is worth settling before the stage 5/6 seams are written against the current shape.

**Stage 3 done** — `3a6b2a1d` on `feat/gitea-520-stage3-raw-scope` (unmerged). Stages 1/2/4 merged earlier in `fbe7403c`. Stage 3 makes the `/raw` device tree addressable by an ACL rule and builds the `scopePaths` producer. It was larger than planned: the design doc described a raw path as the fixed five-tuple `Folder/Driver/Device/TagGroup/Tag`, but folders and tag groups both self-nest, so `NodeScope` had to stop being fixed-arity entirely. Separately, level 1 of the walk gated on a `NamespaceId` that **v3 has no entity for** — it could only ever fail to match, which silently made every sub-cluster grant unreachable while cluster-scoped grants kept working. That level is now the realm, derived from the scope's kind. **Blockers 1, 3 and 4 from this issue are now closed.** Blocker 2 was dissolved by the API-key model in stage 2. ⚠️ **Node ACLs are still NOT enforced.** Nothing constructs a `NodeScope` or calls the evaluator; only `Cluster`-scoped grants confer anything, via `ApiKeyInterimRoleMapper`. That is stages 5 (write / alarms / HistoryRead) and 6 (read / browse / subscribe). This stage makes the model able to *express* what those will enforce. Four defects found by review while doing it, all fixed: #528 #529 #530 #531. Two deferred: #532 #533 — **#533 is worth settling before the stage 5/6 seams are written against the current shape.**
Author
Owner

Stage 3 is merged to master — merge 94990990 (stage-3 commit 3a6b2a1d), pushed.

Two of the four wire-up blockers this issue tracks are now closed:

  1. Raw-realm nodes have no representable scope — NodeScope is (ClusterId, Kind, Segments) with a variable-length path, NodeHierarchyKind carries Uns and Raw, and four Raw* scope kinds were appended for authoring. The old fixed-arity shape could not address a real raw tree at all: folder chains and tag-group chains both nest without bound, so it silently capped at five levels and mislabelled every one.
  2. PermissionTrieBuilder's scopePaths has no producer — AclScopePathResolver is it, built on both sides of the split (EF at the deploy gate, artifact JSON on the node). scopePaths is now required with no fallback; an unresolvable scope is dropped with a diagnostic rather than planted at a trie position the walk can never reach.

Blocker 4 (no session lifecycle) is stage 5's, as before.

Node ACLs are still NOT enforced. The two coarse server-wide gates (WriteOperate, AlarmAck) remain the whole of data-plane authorization; Read/Browse/HistoryRead/CreateMonitoredItems are still ungated for any admitted session. docs/security.md is updated to say so precisely.

Also landed in the same merge, all closed: #532 (Cluster:Roles had no OTOPCUA_ROLES fallback despite two doc sites promising one — the null ClusterId made #528's cross-cluster guard fail open), #533 (three string-identity rules in one authorization path; now one rule — identifiers OrdinalIgnoreCase, path segments Ordinal), and #534 (both libplctag drivers ignored the authored EtherNet/IP port).

Next: stage 5 — enforcement. It is unblocked in both realms now.

**Stage 3 is merged to `master`** — merge `94990990` (stage-3 commit `3a6b2a1d`), pushed. Two of the four wire-up blockers this issue tracks are now closed: 1. ~~Raw-realm nodes have no representable scope~~ — `NodeScope` is `(ClusterId, Kind, Segments)` with a variable-length path, `NodeHierarchyKind` carries `Uns` and `Raw`, and four `Raw*` scope kinds were appended for authoring. The old fixed-arity shape could not address a real raw tree at all: folder chains and tag-group chains both nest without bound, so it silently capped at five levels and mislabelled every one. 3. ~~`PermissionTrieBuilder`'s `scopePaths` has no producer~~ — `AclScopePathResolver` is it, built on both sides of the split (EF at the deploy gate, artifact JSON on the node). `scopePaths` is now required with no fallback; an unresolvable scope is dropped with a diagnostic rather than planted at a trie position the walk can never reach. Blocker 4 (no session lifecycle) is stage 5's, as before. **Node ACLs are still NOT enforced.** The two coarse server-wide gates (`WriteOperate`, `AlarmAck`) remain the whole of data-plane authorization; Read/Browse/HistoryRead/CreateMonitoredItems are still ungated for any admitted session. `docs/security.md` is updated to say so precisely. Also landed in the same merge, all closed: **#532** (`Cluster:Roles` had no `OTOPCUA_ROLES` fallback despite two doc sites promising one — the null ClusterId made #528's cross-cluster guard fail *open*), **#533** (three string-identity rules in one authorization path; now one rule — identifiers `OrdinalIgnoreCase`, path segments `Ordinal`), and **#534** (both libplctag drivers ignored the authored EtherNet/IP port). Next: stage 5 — enforcement. It is unblocked in both realms now.
Author
Owner

Stage 5 done — write, alarms and HistoryRead are enforced behind Security:Acl:Enabled

Branch feat/gitea-520-stage5-enforce, not yet merged. Full solution suite green.

The flag defaults to false, and with it off behaviour is byte-for-byte what it was. With it on, scoped NodeAcl grants decide writes, alarm commands and HistoryRead — the last of which had no authorization check of any kind before this. Read, Browse and CreateMonitoredItems stay ungated in both modes; that is stage 6, and the AdminUI banners stay up until it lands (they now state the conditional truth rather than a flat "not enforced").

The one design decision worth arguing with

The ACL decision replaces the role check; the two are not chained. Chained, a grant would have to confer the server-wide role as well as pass the scope check — and only Cluster-scoped grants confer a role (ApiKeyInterimRoleMapper) — so every narrow grant would have stayed inert and the flag would have looked inert too. Narrow grants working is the whole point of the stage.

That has a testing consequence I want on the record: the load-bearing test is an ALLOW, not a deny. A session carrying no roles at all writes successfully on the strength of its grant. A deny-only test passes against a build where the ACL is never consulted, because the role gate denies too — confirmed by mutation.

Found en route: the principal never reached the session

The authenticator passed it as OpcUaUserAuthResult.DisplayName, but RoleCarryingUserIdentity derives DisplayName from the presented token — which for every key-bearing client is the static username apikey. So the trie's matching key was being dropped at the identity boundary, and the audit ClientUserId on a rejected write recorded apikey for every consuming system. Principal and ApiKeyId are now carried explicitly.

Beyond the stage as written

  • Revocation now reaches an OPEN session. Every decision re-checks the authenticating key against the current snapshot. Without it, a key revoked by a deployment kept its session's authority until the client chose to reconnect — indefinitely, for a machine-to-machine consumer — which would have made the documented "revocation requires a deploy" edge considerably sharper than documented.
  • Blocker 4 is closed, but not as it was framed. UserAuthorizationState is built per call, not per session. The per-session shape is LDAP-inherited: it exists so a long-lived session can re-resolve membership that changed out of band. Nothing can change out of band under API keys, so the generation stamp is the entire invalidation signal and a per-call record picks it up for free, where a cached one would need explicit invalidation to do the same job. IsStale/NeedsRefresh therefore cannot fire on this path and are documented as vestigial rather than left looking load-bearing. Caching earns its keep at stage 6's publish-path seams, and wants a per-(session, node) decision cache — not this type.
  • The trie is built inside DataPlaneAuthorityState.Apply, under the same generation as the snapshot, rather than by a second wiring at the publish actor. An uninstalled trie denies everything, which is indistinguishable from working enforcement until someone checks that an ALLOW is possible at all. Deleting the install turns 13 of the 22 new tests red.
  • The deploy log reports what the build actually dropped. The previous unresolvedScopes count agreed with the builder on one of its four drop reasons; a rule with a blank ScopeId, a retired scope kind, or a scope path not starting at a realm vanished with the log reading zero.

Store widening

Both IApiKeyStore implementations now return the full grant set. The narrowing was redundant — ApiKeyInterimRoleMapper applies the same filter itself — and became a trap once scope was enforced. Pinned by asserting the roles are unchanged, with the two grants deliberately carrying different flags so it cannot pass by accident.

Falsification

Each mutation turns the intended tests red and no others: enforcement never engaging (2), the write seam ignoring the ACL (1 — the allow-side test), the realm dropped from the scope build (1), the revocation check removed (2), the trie never installed (13 of 22).

Not done here

  • Stage 6 — read/browse/subscribe, with per-session decision caching. This is the seam on the hot publish path.
  • The stage 8 live gate has NOT run. Everything above is offline. Enabling this on docker-dev and driving it through Client.CLI is still outstanding, and given every failure path denies, a wiring mistake presents as a total outage of the three gated operations rather than a subtle permission bug — so the gate matters more than usual here.
  • NodePermissions.AlarmRead still has no OpcUaOperation mapping (grantable, unreachable), and ScopeId is still free text until the stage 7 picker.
## Stage 5 done — write, alarms and HistoryRead are enforced behind `Security:Acl:Enabled` Branch `feat/gitea-520-stage5-enforce`, not yet merged. Full solution suite green. **The flag defaults to `false`, and with it off behaviour is byte-for-byte what it was.** With it on, scoped `NodeAcl` grants decide writes, alarm commands and HistoryRead — the last of which had **no authorization check of any kind** before this. Read, Browse and CreateMonitoredItems stay ungated in both modes; that is stage 6, and the AdminUI banners stay up until it lands (they now state the conditional truth rather than a flat "not enforced"). ### The one design decision worth arguing with **The ACL decision replaces the role check; the two are not chained.** Chained, a grant would have to confer the server-wide role *as well as* pass the scope check — and only `Cluster`-scoped grants confer a role (`ApiKeyInterimRoleMapper`) — so every narrow grant would have stayed inert and the flag would have looked inert too. Narrow grants working is the whole point of the stage. That has a testing consequence I want on the record: **the load-bearing test is an ALLOW, not a deny.** A session carrying *no roles at all* writes successfully on the strength of its grant. A deny-only test passes against a build where the ACL is never consulted, because the role gate denies too — confirmed by mutation. ### Found en route: the principal never reached the session The authenticator passed it as `OpcUaUserAuthResult.DisplayName`, but `RoleCarryingUserIdentity` derives `DisplayName` from the presented **token** — which for every key-bearing client is the static username `apikey`. So the trie's matching key was being dropped at the identity boundary, and the audit `ClientUserId` on a rejected write recorded `apikey` for every consuming system. `Principal` and `ApiKeyId` are now carried explicitly. ### Beyond the stage as written - **Revocation now reaches an OPEN session.** Every decision re-checks the authenticating key against the current snapshot. Without it, a key revoked by a deployment kept its session's authority until the client chose to reconnect — indefinitely, for a machine-to-machine consumer — which would have made the documented "revocation requires a deploy" edge considerably sharper than documented. - **Blocker 4 is closed, but not as it was framed.** `UserAuthorizationState` is built **per call, not per session**. The per-session shape is LDAP-inherited: it exists so a long-lived session can re-resolve membership that changed out of band. Nothing can change out of band under API keys, so the generation stamp is the entire invalidation signal and a per-call record picks it up for free, where a cached one would need explicit invalidation to do the same job. `IsStale`/`NeedsRefresh` therefore cannot fire on this path and are documented as vestigial rather than left looking load-bearing. Caching earns its keep at stage 6's publish-path seams, and wants a per-`(session, node)` decision cache — not this type. - **The trie is built inside `DataPlaneAuthorityState.Apply`**, under the same generation as the snapshot, rather than by a second wiring at the publish actor. An uninstalled trie denies everything, which is indistinguishable from working enforcement until someone checks that an ALLOW is possible at all. Deleting the install turns 13 of the 22 new tests red. - **The deploy log reports what the build actually dropped.** The previous `unresolvedScopes` count agreed with the builder on one of its four drop reasons; a rule with a blank `ScopeId`, a retired scope kind, or a scope path not starting at a realm vanished with the log reading zero. ### Store widening Both `IApiKeyStore` implementations now return the full grant set. The narrowing was redundant — `ApiKeyInterimRoleMapper` applies the same filter itself — and became a trap once scope was enforced. Pinned by asserting the roles are unchanged, with the two grants deliberately carrying different flags so it cannot pass by accident. ### Falsification Each mutation turns the intended tests red and no others: enforcement never engaging (2), the write seam ignoring the ACL (1 — the allow-side test), the realm dropped from the scope build (1), the revocation check removed (2), the trie never installed (13 of 22). ### Not done here - **Stage 6** — read/browse/subscribe, with per-session decision caching. This is the seam on the hot publish path. - **The stage 8 live gate has NOT run.** Everything above is offline. Enabling this on docker-dev and driving it through Client.CLI is still outstanding, and given every failure path denies, a wiring mistake presents as a total outage of the three gated operations rather than a subtle permission bug — so the gate matters more than usual here. - `NodePermissions.AlarmRead` still has no `OpcUaOperation` mapping (grantable, unreachable), and `ScopeId` is still free text until the stage 7 picker.
Author
Owner

Stage 5 merged to master — merge 1ccc7400 (branch commit f692f6af), pushed to origin.

This supersedes the "done but unmerged" note in my previous comment. Nothing changed in the tree between that comment and the merge; master was still at 94990990, so the merge is exactly the reviewed branch.

The stage 8 live gate still has NOT run. Security:Acl:Enabled defaults to false, so merging changes no deployed behaviour — but the flag is now flippable on a real node without a code change, and every failure path in the gate denies. A wiring or config mistake therefore presents as a total outage of writes, alarm commands and HistoryRead rather than as a subtle permission bug. Do not turn it on anywhere that matters until the docker-dev gate has run.

Remaining: stage 6 (read/browse/subscribe, with per-(session, node) decision caching on the hot publish path), stage 7 (authoring UI against principals + a RawTree picker for ScopeId, and removal of the AdminUI warning banners), stage 8 (docs, ScadaBridge key hand-off, the live gate).

Two known inconsistencies carried forward, neither blocking: NodePermissions.AlarmRead has no OpcUaOperation mapping, so it is grantable but unreachable; and NodeAcl.ScopeId stays free text until stage 7 ships the picker.

**Stage 5 merged to `master`** — merge `1ccc7400` (branch commit `f692f6af`), pushed to origin. This supersedes the "done but unmerged" note in my previous comment. Nothing changed in the tree between that comment and the merge; `master` was still at `94990990`, so the merge is exactly the reviewed branch. **The stage 8 live gate still has NOT run.** `Security:Acl:Enabled` defaults to `false`, so merging changes no deployed behaviour — but the flag is now flippable on a real node without a code change, and every failure path in the gate denies. A wiring or config mistake therefore presents as a total outage of writes, alarm commands and HistoryRead rather than as a subtle permission bug. Do not turn it on anywhere that matters until the docker-dev gate has run. Remaining: stage 6 (read/browse/subscribe, with per-`(session, node)` decision caching on the hot publish path), stage 7 (authoring UI against principals + a `RawTree` picker for `ScopeId`, and removal of the AdminUI warning banners), stage 8 (docs, ScadaBridge key hand-off, the live gate). Two known inconsistencies carried forward, neither blocking: `NodePermissions.AlarmRead` has no `OpcUaOperation` mapping, so it is grantable but unreachable; and `NodeAcl.ScopeId` stays free text until stage 7 ships the picker.
Author
Owner

Stage 6 merged to master — merge d4389f57, pushed. Stage 8 live gate PASSED on docker-dev the same day.

Enforcement is now complete behind Security:Acl:Enabled (still default false): read (every attribute, not just Value), browse, TranslateBrowsePath, CreateMonitoredItems, SubscribeToEvents and SetMonitoringMode all consult scoped grants.

The two things that needed live verification

Revoking a grant mid-subscription, as a transcript:

13:49:39  SiteAOnly = 53   (0x00000000)   streaming ~1/s
13:49:40  SiteAOnly =      (0x801F0000)   revoking deploy lands; the client is TOLD
          ... 25 seconds of silence, ~25 updates that never arrived ...
13:50:05  SiteAOnly = 157  (0x00000000)   restoring deploy; the feed resumes by itself

53 -> 157 across the gap: the device counter ran throughout, so delivery stopped, not the source.

Browse filtering, A/B against the same artifact: gated node shows plc only; control node (flag off) shows plc and plc2.

Design decisions worth arguing with

  • Browse needed ancestor visibility. The trie inherits downward and a client reaches a deep node by browsing the ones above it, so without this every grant narrower than the cluster is authored, shipped and unreachable. It is a separate non-inheriting marker, not a synthesized Browse grant on each ancestor — the additive walk would carry that onto every sibling subtree. Accepted consequence: a narrow Browse grant exposes the names of its ancestor folders.
  • A denied item is detached from its node fan-out, NOT set MonitoringMode.Disabled. Disabling discards the item queued notifications, so the client is silenced without being told why. The over-the-wire test caught this; the unit path reports the item as stood down either way. Detaching is also reversible, so a restored grant resumes the feed with no client action.
  • The decision cache holds the grant walk, not the decision. Grants are fixed for a generation; a key expiry moves with the clock, so caching the whole decision would let an expired key work until the next deploy.

Three defects found en route, all the same shape

An object-typed SDK parameter that the wrong cast reads as null, disabling a gate silently and compiling cleanly:

  1. ContinuationPoint.NodeToBrowse is a NodeHandle, not a NodeState — the browse source gate was skipped.
  2. SubscribeToEvents sourceId likewise — an ungranted session could have subscribed to an equipment event notifier and received every alarm it raises, while every value on it was refused.
  3. UserAuthorizationState staleness ceiling couples the authorizer clock to the evaluator, and any divergence denies everything with a verdict indistinguishable from "no grant matched". Stage 5 claim that these predicates cannot fire was accidentally true; now disabled explicitly.

Not covered by the gate

A UNS-realm grant (the rig SITE-A cluster has no UNS equipment) and alarm-command scoping (no alarms there). Both are covered offline; neither has been seen live.

Consumer-visible, and the umbrella index was wrong

With the flag on, a read-only consumer now needs a Read grant. The scadaproj index previously told ScadaBridge that reads and subscriptions were ungated in either mode — corrected and pushed (555d1b4).

Remaining: stage 7 (ACL authoring UI against principals, a RawTree picker for ScopeId, and removing the two AdminUI warning banners). Known inconsistencies carried forward: NodePermissions.AlarmRead has no OpcUaOperation mapping so it is grantable but unreachable, and NodeAcl.ScopeId stays free text until that picker.

**Stage 6 merged to `master`** — merge `d4389f57`, pushed. **Stage 8 live gate PASSED** on docker-dev the same day. Enforcement is now complete behind `Security:Acl:Enabled` (still default `false`): read (every attribute, not just Value), browse, `TranslateBrowsePath`, `CreateMonitoredItems`, `SubscribeToEvents` and `SetMonitoringMode` all consult scoped grants. ### The two things that needed live verification Revoking a grant mid-subscription, as a transcript: ``` 13:49:39 SiteAOnly = 53 (0x00000000) streaming ~1/s 13:49:40 SiteAOnly = (0x801F0000) revoking deploy lands; the client is TOLD ... 25 seconds of silence, ~25 updates that never arrived ... 13:50:05 SiteAOnly = 157 (0x00000000) restoring deploy; the feed resumes by itself ``` `53 -> 157` across the gap: the device counter ran throughout, so **delivery** stopped, not the source. Browse filtering, A/B against the same artifact: gated node shows `plc` only; control node (flag off) shows `plc` **and** `plc2`. ### Design decisions worth arguing with - **Browse needed ancestor visibility.** The trie inherits downward and a client reaches a deep node by browsing the ones above it, so without this every grant narrower than the cluster is authored, shipped and unreachable. It is a separate non-inheriting marker, **not** a synthesized `Browse` grant on each ancestor — the additive walk would carry that onto every sibling subtree. Accepted consequence: a narrow Browse grant exposes the *names* of its ancestor folders. - **A denied item is detached from its node fan-out, NOT set `MonitoringMode.Disabled`.** Disabling *discards the item queued notifications*, so the client is silenced without being told why. The over-the-wire test caught this; the unit path reports the item as stood down either way. Detaching is also reversible, so a restored grant resumes the feed with no client action. - **The decision cache holds the grant walk, not the decision.** Grants are fixed for a generation; a key expiry moves with the clock, so caching the whole decision would let an expired key work until the next deploy. ### Three defects found en route, all the same shape An `object`-typed SDK parameter that the wrong cast reads as `null`, disabling a gate silently and compiling cleanly: 1. `ContinuationPoint.NodeToBrowse` is a `NodeHandle`, not a `NodeState` — the browse **source** gate was skipped. 2. `SubscribeToEvents` `sourceId` likewise — an ungranted session could have subscribed to an equipment event notifier and received every alarm it raises, while every value on it was refused. 3. `UserAuthorizationState` staleness ceiling couples the authorizer clock to the evaluator, and any divergence denies **everything** with a verdict indistinguishable from "no grant matched". Stage 5 claim that these predicates cannot fire was accidentally true; now disabled explicitly. ### Not covered by the gate A **UNS-realm grant** (the rig SITE-A cluster has no UNS equipment) and **alarm-command scoping** (no alarms there). Both are covered offline; neither has been seen live. ### Consumer-visible, and the umbrella index was wrong With the flag **on**, a read-only consumer now needs a `Read` grant. The scadaproj index previously told ScadaBridge that reads and subscriptions were ungated *in either mode* — corrected and pushed (`555d1b4`). Remaining: **stage 7** (ACL authoring UI against principals, a `RawTree` picker for `ScopeId`, and removing the two AdminUI warning banners). Known inconsistencies carried forward: `NodePermissions.AlarmRead` has no `OpcUaOperation` mapping so it is grantable but unreachable, and `NodeAcl.ScopeId` stays free text until that picker.
Author
Owner

Stage 7 done — #520 is complete. master 3796c0bf (merge), work commit d542ab5b. Pushed.

What landed

Grants are authored from a scope picker over the cluster's real raw + UNS trees, and it supplies the scope kind and the scope target together. Before this the two were an unrelated dropdown and a free-text box, so RawDevice + a TagId was a well-formed row: it saved, it deployed, and it authorized nobody. Because the trees are rooted at the grant's own cluster and each node carries its own kind, four of the deploy gate's five rules become unauthorable by construction rather than merely rejected later — NodeAclScopeCrossCluster, NodeAclScopeIdUnexpected, NodeAclScopeIdMissing, NodeAclScopeKindRetired.

IAclScopeService.CheckScopeAsync mirrors DraftValidator.ValidateNodeAclScopes rule for rule and blocks the save when a scope resolves to nothing; the list page marks rotted rows unresolved with the reason. The principal field suggests known principals and reports how many live keys reach the one typed — reaching nobody warns rather than blocks, since authoring ahead of minting a key is legitimate and a typo is not, and the two are indistinguishable to the server.

Departures from the stage line, all deliberate

  • The banners were reshaped, not removed. "Remove the two 'not enforced' banners" assumed that was all they said, which stage 6 falsified. What is still true is that enforcement is a per-node flag the AdminUI cannot see, so a narrow grant genuinely confers nothing on a default-configured node. Deleting the banner would have swapped one wrong impression for another.
  • A Cluster row produces a NULL ScopeId. The deploy gate rejects a Cluster grant carrying one, so a picker that helpfully filled in the cluster id would author rows that cannot be deployed.
  • AclScopeTree is a new component, not a third mode on RawTree. RawTree already carries the /raw editing tree and the equipment reference picker; a third mode would put this feature's bugs on the device-authoring page, and with no bUnit here the only thing that catches that is someone noticing. It also renders a unified node type, which is what lets one component serve both hierarchies.
  • UNS tags are not offered. A UNS signal is a UnsTagReference, and a Tag grant already covers the raw node and every UNS projection of it — a UNS tag row would read narrower than the grant it creates.

Found en route

The ACL editor had a DataAnnotationsValidator and no ValidationSummary, so an invalid NodeAclId made Create a silent no-op. Present since the page was written; same failure mode as #505.

Verification

30 tests, falsified with six production mutations — each turned exactly the intended tests red and no others. Full AdminUI suite 999/999.

Live gate PASSED on docker-dev, 11 legs. Both central containers asserted on the same freshly-built image by container id (the stale-central-2 trap from the stage-6 gate). The pair that matters:

  • Leg 8 — saving a rotted row is blocked: "Scope: No device in this configuration has the id 'SITEA-dev-DELETED'…"
  • Leg 11 — the same row, deployed: HTTP 422 NodeAclScopeUnresolved, and the gate's message named DriverInstance after the row was re-pointed at RawDriver, confirming the kind is honoured on both sides.

That pair is what the claim "the page mirrors the deploy gate" reduces to. Legs 5 and 9 are the positive controls — without them the gate passes against a page that refuses everything, the same shape stage 6's resolvedScopes=N leg exists to catch. Leg 10: a grant authored through the picker deploys Sealed.

Not covered: UNS-realm scope selection — SITE-A has no UNS areas, so the picker's UNS column correctly showed "no UNS areas yet". UnsArea/UnsLine/Equipment are covered offline only.

Rig restored to zero ACL rows with a further sealed deployment.

Still open, deliberately

  • NodePermissions.AlarmRead has no OpcUaOperation mapping — grantable but unreachable. Unchanged by this stage.
  • NodeAcl.ScopeId still has no FK. It names a row in one of eight tables chosen by ScopeKind, which is not a foreign key any schema can express; the existence check is the substitute, now at both authoring and deploy time.

Docs: docs/security.md §"Authoring grants", docs/plans/2026-07-31-apikey-acl-enforcement-design.md (stage 7 + its live gate), CLAUDE.md. Umbrella index updated on scadaproj/main (7a6f997).

One stale line fixed in docs/security.md while here: it still claimed reads, browses and subscriptions "stay ungated in either mode", which stage 6 had already falsified elsewhere in the same document.

**Stage 7 done — #520 is complete.** `master` `3796c0bf` (merge), work commit `d542ab5b`. Pushed. ### What landed Grants are authored from a **scope picker** over the cluster's real raw + UNS trees, and it supplies the **scope kind and the scope target together**. Before this the two were an unrelated dropdown and a free-text box, so `RawDevice` + a TagId was a well-formed row: it saved, it deployed, and it authorized nobody. Because the trees are rooted at the grant's own cluster and each node carries its own kind, four of the deploy gate's five rules become **unauthorable by construction** rather than merely rejected later — `NodeAclScopeCrossCluster`, `NodeAclScopeIdUnexpected`, `NodeAclScopeIdMissing`, `NodeAclScopeKindRetired`. `IAclScopeService.CheckScopeAsync` mirrors `DraftValidator.ValidateNodeAclScopes` rule for rule and **blocks the save** when a scope resolves to nothing; the list page marks rotted rows `unresolved` with the reason. The principal field suggests known principals and reports how many live keys reach the one typed — reaching nobody **warns rather than blocks**, since authoring ahead of minting a key is legitimate and a typo is not, and the two are indistinguishable to the server. ### Departures from the stage line, all deliberate - **The banners were reshaped, not removed.** "Remove the two 'not enforced' banners" assumed that was all they said, which stage 6 falsified. What is still true is that enforcement is a **per-node** flag the AdminUI cannot see, so a narrow grant genuinely confers nothing on a default-configured node. Deleting the banner would have swapped one wrong impression for another. - **A `Cluster` row produces a NULL `ScopeId`.** The deploy gate rejects a Cluster grant carrying one, so a picker that helpfully filled in the cluster id would author rows that cannot be deployed. - **`AclScopeTree` is a new component, not a third mode on `RawTree`.** `RawTree` already carries the `/raw` editing tree and the equipment reference picker; a third mode would put this feature's bugs on the device-authoring page, and with no bUnit here the only thing that catches that is someone noticing. It also renders a unified node type, which is what lets one component serve both hierarchies. - **UNS tags are not offered.** A UNS signal is a `UnsTagReference`, and a `Tag` grant already covers the raw node *and* every UNS projection of it — a UNS tag row would read narrower than the grant it creates. ### Found en route The ACL editor had a `DataAnnotationsValidator` and **no `ValidationSummary`**, so an invalid `NodeAclId` made Create a silent no-op. Present since the page was written; same failure mode as #505. ### Verification 30 tests, falsified with six production mutations — each turned exactly the intended tests red and no others. Full AdminUI suite 999/999. **Live gate PASSED on docker-dev, 11 legs.** Both central containers asserted on the same freshly-built image *by container id* (the stale-central-2 trap from the stage-6 gate). The pair that matters: - **Leg 8** — saving a rotted row is blocked: *"Scope: No device in this configuration has the id 'SITEA-dev-DELETED'…"* - **Leg 11** — the same row, deployed: **HTTP 422 `NodeAclScopeUnresolved`**, and the gate's message named `DriverInstance` after the row was re-pointed at `RawDriver`, confirming the *kind* is honoured on both sides. That pair is what the claim "the page mirrors the deploy gate" reduces to. Legs 5 and 9 are the positive controls — without them the gate passes against a page that refuses everything, the same shape stage 6's `resolvedScopes=N` leg exists to catch. Leg 10: a grant authored through the picker deploys `Sealed`. **Not covered:** UNS-realm scope selection — SITE-A has no UNS areas, so the picker's UNS column correctly showed "no UNS areas yet". `UnsArea`/`UnsLine`/`Equipment` are covered offline only. Rig restored to zero ACL rows with a further sealed deployment. ### Still open, deliberately - `NodePermissions.AlarmRead` has no `OpcUaOperation` mapping — grantable but unreachable. Unchanged by this stage. - `NodeAcl.ScopeId` still has **no FK**. It names a row in one of eight tables chosen by `ScopeKind`, which is not a foreign key any schema can express; the existence check is the substitute, now at both authoring and deploy time. Docs: `docs/security.md` §"Authoring grants", `docs/plans/2026-07-31-apikey-acl-enforcement-design.md` (stage 7 + its live gate), `CLAUDE.md`. Umbrella index updated on `scadaproj/main` (`7a6f997`). One stale line fixed in `docs/security.md` while here: it still claimed reads, browses and subscriptions "stay ungated in either mode", which stage 6 had already falsified elsewhere in the same document.
Author
Owner

The UNS gap in the stage-7 gate is closed. master a88cefd5.

The first pass could not exercise UNS-realm scope selection — SITE-A had no UNS, so the picker's UNS column correctly showed "no UNS areas yet" and only the namespace row was selectable. Authored one on docker-dev (area → line → equipment) and re-ran that part.

# Leg Result
12 UNS column offers area / line / equipment ✅ UNSAREA / UNSLINE / EQUIPMENT, eager
13 Selecting an equipment resolves ✅ EQUIPMENT + EQ-f3dd0e8e27a4, "✓ Equipment 'packer-1' in cluster 'SITE-A'"
14 A UNS-realm grant deploys ✅ Sealed
15 Cross-cluster caught at both ends ✅ re-pointed at MAIN's equipment → list marks UNRESOLVED naming MAIN; deploy → HTTP 422 NodeAclScopeCrossCluster

Leg 15 is the one worth keeping. It is the only rejection whose id is real, so a check that asked only "does a row with this id exist?" would accept it and confer authority across a cluster boundary. Both ends catch it and both name the owning cluster.

Every stage-7 claim is now covered live. Nothing outstanding on this issue.


Filed en route: #535. Authoring that UNS hit the same defect class this stage just closed, in a different page: the /uns area and line modals do not validate the UNS segment format, so Packaging and Line 1 saved without complaint and then failed the deploy with UnsSegmentInvalid. Only the equipment form carries the ^[a-z0-9-]{1,32}$ rule. A deployment snapshots the whole database, so one badly-named area blocks every cluster's deploy. Out of scope here, not fixed.

**The UNS gap in the stage-7 gate is closed.** `master` `a88cefd5`. The first pass could not exercise UNS-realm scope selection — SITE-A had no UNS, so the picker's UNS column correctly showed "no UNS areas yet" and only the namespace row was selectable. Authored one on docker-dev (area → line → equipment) and re-ran that part. | # | Leg | Result | |---|---|---| | 12 | UNS column offers area / line / equipment | ✅ `UNSAREA` / `UNSLINE` / `EQUIPMENT`, eager | | 13 | Selecting an equipment resolves | ✅ `EQUIPMENT` + `EQ-f3dd0e8e27a4`, "✓ Equipment 'packer-1' in cluster 'SITE-A'" | | 14 | A UNS-realm grant deploys | ✅ `Sealed` | | 15 | Cross-cluster caught at both ends | ✅ re-pointed at MAIN's equipment → list marks `UNRESOLVED` naming `MAIN`; deploy → HTTP 422 `NodeAclScopeCrossCluster` | **Leg 15 is the one worth keeping.** It is the only rejection whose id is *real*, so a check that asked only "does a row with this id exist?" would accept it and confer authority across a cluster boundary. Both ends catch it and both name the owning cluster. Every stage-7 claim is now covered live. Nothing outstanding on this issue. --- **Filed en route: #535.** Authoring that UNS hit the same defect class this stage just closed, in a different page: the `/uns` **area** and **line** modals do not validate the UNS segment format, so `Packaging` and `Line 1` saved without complaint and then failed the deploy with `UnsSegmentInvalid`. Only the equipment form carries the `^[a-z0-9-]{1,32}$` rule. A deployment snapshots the whole database, so one badly-named area blocks **every** cluster's deploy. Out of scope here, not fixed.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#520