fix(security): id-form credential references match by id only; id-shaped driver names refused (#849 review)

A driver named 'drv' (env form DRV) matched every sibling's id-form variable env:<PREFIX>DRV_<12HEX>_... as 'another instance', breaking the recommended id convention fleet-wide - including for a name that pre-dates the rule. ReferenceNames is split into ReferenceNamesId / ReferenceNamesName; a reference whose instance part is id-shaped (secret: DRV-<12 hex>, env: DRV_<12 HEX>[_...]) is matched by id only. DescribeNameChangeProblemsAsync also refuses new names whose env form is DRV or starts with an id form (IsReservedInstanceName) at create, /raw rename and config-modal update. Co-Authored-By: Claude <noreply@anthropic.com>
2026-11-11 06:35:52 -05:00
parent 780da7528a
commit d68dff0946
5 changed files with 186 additions and 16 deletions
@@ -89,7 +89,7 @@ public static class DriverInstanceIdentities
    /// <summary>
    ///     lmxopcua#848: the problems of giving a driver the name <paramref name="newName"/> (create, or a rename via the
    ///     /raw rename or the driver config modal). Two rules, both about a name being the instance part of a stored
-    ///     credential reference (<see cref="DriverCredentialPolicy.ReferenceNames"/>):
+    ///     credential reference (<see cref="DriverCredentialPolicy.ReferenceNamesName"/>):
    ///     <list type="bullet">
    ///         <item>
    ///             <b>Claim</b> — the new name may not be named by a reference STORED on another same-type driver (its
@@ -123,6 +123,19 @@ public static class DriverInstanceIdentities
        if (!DriverCredentialPolicy.TryGet(driverType, out var rules)) return [];
        if (currentName is not null && string.Equals(currentName, newName, StringComparison.OrdinalIgnoreCase)) return [];

+        // #849 review: a name in the system-generated id form (env form 'DRV' or 'DRV_<12 HEX>…') would shadow the id
+        // convention every driver's own references are recommended to use. The matcher already ignores such a name for
+        // id-shaped references (so a pre-existing one breaks nothing); this keeps new ones from being created at all.
+        if (DriverCredentialPolicy.IsReservedInstanceName(newName))
+        {
+            return
+            [
+                $"The name '{newName}' has the form of a system-generated driver id ('{DriverCredentialPolicy.DriverIdPrefix}' or "
+                + $"'{DriverCredentialPolicy.DriverIdPrefix}-<12 hex>…'), which {rules.Scope.Label} credential references use to name a "
+                + "driver by its id — choose another name (lmxopcua#848/#849).",
+            ];
+        }
+
        // Same shape as LoadAsync: the type filter stays client-side (case-insensitive), the device read is a plain IN.
        var drivers = (await db.DriverInstances.AsNoTracking()
                .Select(d => new { d.DriverInstanceId, d.Name, d.DriverType, d.DriverConfig })
@@ -154,7 +167,7 @@ public static class DriverInstanceIdentities
                var where = device is null ? "" : $" device '{device}'";
                foreach (var reference in DriverCredentialPolicy.CollectInstanceReferences(driverType, json))
                {
-                    if (!own && DriverCredentialPolicy.ReferenceNames(reference, newName))
+                    if (!own && DriverCredentialPolicy.ReferenceNamesName(reference, newName))
                    {
                        problems.Add(
                            $"The name '{newName}' is the instance part of the credential reference {reference.FieldPath} on "
@@ -163,9 +176,9 @@ public static class DriverInstanceIdentities
                            + "Choose another name, or move that reference first (lmxopcua#848).");
                    }
                    else if (own && currentName is not null
-                             && DriverCredentialPolicy.ReferenceNames(reference, currentName)
-                             && !DriverCredentialPolicy.ReferenceNames(reference, ownDriverInstanceId)
-                             && !DriverCredentialPolicy.ReferenceNames(reference, newName))
+                             && DriverCredentialPolicy.ReferenceNamesName(reference, currentName)
+                             && !DriverCredentialPolicy.ReferenceNamesId(reference, ownDriverInstanceId)
+                             && !DriverCredentialPolicy.ReferenceNamesName(reference, newName))
                    {
                        problems.Add(
                            $"This driver's credential reference {reference.FieldPath}{where} is named after its current name "
@@ -236,7 +236,7 @@ public static class DriverCredentialPolicy
    ///     <c>env:OTOPCUA_GALAXY_&lt;REST&gt;</c> ⇒ <c>REST</c>. The per-instance convention is
    ///     <c>env:&lt;PREFIX&gt;&lt;INSTANCE&gt;_…</c>, INSTANCE being a driver id or name in
    ///     <see cref="NormalizeEnvironmentIdentifier"/> form; the part NAMES an instance when it equals that form or starts
-    ///     with it followed by <c>_</c> (see <see cref="ReferenceNames"/>). Null for anything but an in-scope <c>env:</c>.
+    ///     with it followed by <c>_</c> (see <see cref="ReferenceNamesId"/> / <see cref="ReferenceNamesName"/>). Null for anything but an in-scope <c>env:</c>.
    /// </summary>
    /// <param name="value">The authored field value.</param>
    /// <param name="scope">The driver's scope.</param>
@@ -269,15 +269,81 @@ public static class DriverCredentialPolicy
        });
    }

+    /// <summary>The prefix of every system-generated driver instance id (<c>DRV-</c> + 12 hex, <c>RawTreeService.NewId</c>).</summary>
+    public const string DriverIdPrefix = "DRV";
+
+    private const int DriverIdHexLength = 12;
+
    /// <summary>
-    ///     Whether an instance reference (<see cref="CollectInstanceReferences"/>) names the driver id or name
-    ///     <paramref name="identifier"/>: a <c>secret:</c> segment equal to it (case-insensitive), or an <c>env:</c> part
-    ///     equal to its <see cref="NormalizeEnvironmentIdentifier"/> form or starting with that form plus <c>_</c>.
+    ///     Whether an instance reference (<see cref="CollectInstanceReferences"/>) names the driver id
+    ///     <paramref name="driverInstanceId"/>: a <c>secret:</c> segment equal to it (case-insensitive), or an <c>env:</c>
+    ///     part equal to its <see cref="NormalizeEnvironmentIdentifier"/> form or starting with that form plus <c>_</c>.
    /// </summary>
    /// <param name="reference">The reference.</param>
-    /// <param name="identifier">A driver id or name.</param>
+    /// <param name="driverInstanceId">A driver instance id.</param>
    /// <returns>Whether it names it.</returns>
-    public static bool ReferenceNames(InstanceReference reference, string? identifier)
+    public static bool ReferenceNamesId(InstanceReference reference, string? driverInstanceId) =>
+        Matches(reference, driverInstanceId);
+
+    /// <summary>
+    ///     Whether an instance reference names the driver NAME <paramref name="name"/> — as <see cref="ReferenceNamesId"/>,
+    ///     except that a reference whose instance part has the shape of a system-generated id
+    ///     (<see cref="IsIdShaped"/>: <c>secret:…/DRV-&lt;12 hex&gt;/…</c>, <c>env:&lt;PREFIX&gt;DRV_&lt;12 HEX&gt;[_…]</c>) is
+    ///     matched by id only. Otherwise a driver NAMED <c>drv</c> (env form <c>DRV</c>) would "own" the id-form variable of
+    ///     every other driver of its type, and the recommended id convention would break fleet-wide — including for a
+    ///     name that pre-dates the rule (lmxopcua#849 review).
+    /// </summary>
+    /// <param name="reference">The reference.</param>
+    /// <param name="name">A driver name.</param>
+    /// <returns>Whether it names it.</returns>
+    public static bool ReferenceNamesName(InstanceReference reference, string? name) =>
+        !IsIdShaped(reference) && Matches(reference, name);
+
+    /// <summary>
+    ///     Whether the reference's instance part has the shape of a system-generated driver id: a <c>secret:</c> segment
+    ///     <c>DRV-&lt;12 hex&gt;</c> (case-insensitive), or an <c>env:</c> part <c>DRV_&lt;12 HEX&gt;</c> alone or followed by <c>_</c>.
+    /// </summary>
+    /// <param name="reference">The reference.</param>
+    /// <returns>Whether it is id-shaped.</returns>
+    public static bool IsIdShaped(InstanceReference reference) =>
+        reference.Kind == CredentialReferenceKind.Secret
+            ? reference.Part.Length == DriverIdPrefix.Length + 1 + DriverIdHexLength && StartsWithIdForm(reference.Part, '-', ignoreCase: true)
+            : StartsWithIdForm(reference.Part, '_', ignoreCase: false)
+              && (reference.Part.Length == DriverIdPrefix.Length + 1 + DriverIdHexLength
+                  || reference.Part[DriverIdPrefix.Length + 1 + DriverIdHexLength] == '_');
+
+    /// <summary>
+    ///     lmxopcua#849 review: a driver NAME that may not be given to a credential-bearing driver — one whose environment
+    ///     form (<see cref="NormalizeEnvironmentIdentifier"/>) is <c>DRV</c> or starts with an id form <c>DRV_&lt;12 HEX&gt;</c>.
+    ///     Such a name would collide with the system-generated id convention every driver's own references are
+    ///     recommended to use.
+    /// </summary>
+    /// <param name="name">The proposed name.</param>
+    /// <returns>Whether the name is reserved.</returns>
+    public static bool IsReservedInstanceName(string? name)
+    {
+        var env = NormalizeEnvironmentIdentifier(name);
+        return env == DriverIdPrefix
+               || IsIdShaped(new InstanceReference("", CredentialReferenceKind.Environment, env));
+    }
+
+    private static bool StartsWithIdForm(string part, char separator, bool ignoreCase)
+    {
+        var length = DriverIdPrefix.Length + 1 + DriverIdHexLength;
+        if (part.Length < length
+            || !part.StartsWith(DriverIdPrefix, ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal)
+            || part[DriverIdPrefix.Length] != separator)
+            return false;
+        for (var i = DriverIdPrefix.Length + 1; i < length; i++)
+        {
+            var c = part[i];
+            if (!(char.IsAsciiDigit(c) || c is >= 'A' and <= 'F' || (ignoreCase && c is >= 'a' and <= 'f'))) return false;
+        }
+
+        return true;
+    }
+
+    private static bool Matches(InstanceReference reference, string? identifier)
    {
        if (string.IsNullOrEmpty(identifier)) return false;
        if (reference.Kind == CredentialReferenceKind.Secret)
@@ -300,7 +366,7 @@ public static class DriverCredentialPolicy
        CollectInstanceReferences(driverType, json).Count > 0;

    /// <summary>
-    ///     lmxopcua#835/#849: inside one namespace, a reference that names (see <see cref="ReferenceNames"/>) a DIFFERENT
+    ///     lmxopcua#835/#849: inside one namespace, a reference that names (see <see cref="ReferenceNamesId"/> / <see cref="ReferenceNamesName"/>) a DIFFERENT
    ///     existing instance of the same driver type — by its <see cref="DriverInstanceIdentity.DriverInstanceId"/> or its
    ///     <see cref="DriverInstanceIdentity.Name"/> — is refused: otherwise a Designer could create (or Test Connect /
    ///     browse) a driver that points at their own endpoint with <c>secret:galaxy/&lt;other-instance&gt;/apikey</c> (or
@@ -333,7 +399,7 @@ public static class DriverCredentialPolicy
            .ToList();
        foreach (var reference in references)
        {
-            var other = others.FirstOrDefault(o => ReferenceNames(reference, o.DriverInstanceId) || ReferenceNames(reference, o.Name));
+            var other = others.FirstOrDefault(o => ReferenceNamesId(reference, o.DriverInstanceId) || ReferenceNamesName(reference, o.Name));
            if (other is null) continue;
            problems.Add(reference.Kind == CredentialReferenceKind.Secret
                ? $"{reference.FieldPath} names a secret under '{reference.Part}/', which is another {rules.Scope.Label} driver "