Skip to content

GDS: pin SecurityAdmin-only delivery of ApplicationRegistrationChangedAuditEvents; document CTT defect C57 - #4693

Merged
marcschier merged 3 commits into
masterfrom
romanett/gds-audit-event-receive-permissions
Oct 7, 2026
Merged

marcschier merged 3 commits into
masterfrom
romanett/gds-audit-event-receive-permissions

Conversation

@romanett

@romanett romanett commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Description

CTT 1.05.513 (patched scripts) GDS Application Directory 011.js, 018.js and 028.js fail on master (fa06c66) with "Step 3: Did not receive any Events after calling Publish." and passed on efad0dd.

Root cause

7212607 (#4675, Part 3 compliance), not #4668. Since #4675 the server enforces ReceiveEvents on the EventType node (OPC 10000-3 §8.55: a Client only receives an Event if it has ReceiveEvents on its EventType and SourceNode). The GDS NodeSet grants it on ApplicationRegistrationChangedAuditEventType (AuditEventDefaultPermissions) only to SecurityAdmin. The CTT test cases subscribe on the Session of the GDS AdministrativeUser, a DiscoveryAdmin, so the server correctly withholds the events. The audit events are still raised and delivered: the RegisterApplication change to OnCallAsync in #4668 does not affect them.

This is a CTT defect, so the server is unchanged. It is documented as C57 in plans/ctt-issues.md and filed as Mantis 11482. The fix is for the scripts to subscribe on a second Session of the CU's SecurityAdminUser.

Changes

  • tests/Opc.Ua.Gds.Tests/GdsApplicationRegistrationAuditEventTests.cs (new): a DiscoveryAdmin calls RegisterApplication, UpdateApplication and UnregisterApplication. A SecurityAdmin subscriber on Server.EventNotifier must receive all three ApplicationRegistrationChangedAuditEvents with SourceNode = Directory and the matching MethodId. A DiscoveryAdmin subscriber must receive none of them.
  • plans/ctt-issues.md: C57 (Mantis 11482).

Verification

  • New test passes on net10.0 (~1 s); the test project builds on net48.
  • CTT p14-gds against an unchanged master server (fa06c66) with the C57 script patch (second Session as sysadmin for the event subscription), compared with efad0dd using the unpatched scripts:
    • Application Directory 011/018/028: Error → Pass.
    • AliasName Discovery 001 Pass, 002 Warning, 004 Pass.
    • Only other differences: two response-timestamp timing Warnings (App Directory 023 ActivateSession and Query Applications 002 CreateSession, > 100 ms).
  • CTT p12-auditing against fa06c66: identical to the baseline (21 Pass, 5 Skipped, 81 NotImpl, 0 errors).

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 7, 2026 05:02
…dAuditEvents; document CTT C57

Since #4675 the server enforces ReceiveEvents on the EventType node
(OPC 10000-3 8.55). The GDS NodeSet grants it on
ApplicationRegistrationChangedAuditEventType (AuditEventDefaultPermissions)
only to SecurityAdmin, so a DiscoveryAdmin that calls RegisterApplication,
UpdateApplication or UnregisterApplication no longer receives the audit
events. CTT GDS Application Directory 011/018/028 subscribe as the
DiscoveryAdmin user and fail since; documented as CTT defect C57.

The new test calls the three Methods as DiscoveryAdmin and asserts that a
SecurityAdmin subscriber on Server.EventNotifier receives all three audit
events with SourceNode = Directory, and that a DiscoveryAdmin subscriber
receives none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The behavior is adequately tested and documented, with only a minor class-sealing convention issue.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds regression coverage confirming GDS registration audit events are delivered only to SecurityAdmin sessions and documents CTT defect C57.

Changes:

  • Tests Register, Update, and Unregister audit-event delivery by role.
  • Documents the required CTT session correction.
File Description
tests/​Opc.Ua.Gds.Tests/​GdsApplicationRegistrationAuditEventTests.cs Adds role-based audit-event integration coverage.
plans/​ctt-issues.md Documents CTT defect C57 and its workaround.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/Opc.Ua.Gds.Tests/GdsApplicationRegistrationAuditEventTests.cs Outdated
romanett and others added 2 commits October 7, 2026 07:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Code coverage

✅ Coverage gate passed.

Check Result Threshold
✅ Project line rate 88.59% (305303/344638 lines) >= 70.00%
✅ Project branch rate 79.01% >= 60.00%
ℹ️ Patch coverage no changed C# files -
ℹ️ Baseline delta (advisory) +14.99 pp 73.60% recorded

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 87.72%, branch 78.21%.

@codecov

codecov Bot commented Oct 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.82%. Comparing base (fa06c66) to head (7220e6a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4693      +/-   ##
==========================================
- Coverage   82.82%   82.82%   -0.01%     
==========================================
  Files        2262     2262              
  Lines      344638   344638              
  Branches    60051    60051              
==========================================
- Hits       285448   285441       -7     
- Misses      39161    39169       +8     
+ Partials    20029    20028       -1     
Flag Coverage Δ
actions 82.82% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants