From 8adb09a0833096947f8cb647fc4687b489cc0673 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 2 Sep 2026 12:43:21 -0400 Subject: [PATCH] feat(delmia-notifier): optional ScadaBridge:ApiKey in appsettings.json as a fallback to SCADABRIDGE_API_KEY (env still wins); README/env inventory/design row updated The 2026-06-26 design kept the key env-only. The file is now an explicit fallback: SCADABRIDGE_API_KEY (non-blank) -> ScadaBridge:ApiKey (non-blank, trimmed) -> not configured. Env-only stays the recommended production posture; the failure message names both sources. Claude-Session: https://claude.ai/code/session_013L3XdmVii9i5sY4FChU8pB --- ...026-06-26-delmia-recipe-notifier-design.md | 2 +- env_variables.md | 2 +- .../ConfigLoader.cs | 17 ++++++--- .../NotifierConfig.cs | 3 ++ .../Program.cs | 6 ++-- .../README.md | 19 +++++----- .../appsettings.json | 3 +- .../ConfigLoaderTests.cs | 35 +++++++++++++++++-- 8 files changed, 65 insertions(+), 22 deletions(-) diff --git a/docs/plans/2026-06-26-delmia-recipe-notifier-design.md b/docs/plans/2026-06-26-delmia-recipe-notifier-design.md index 7b4b2f92..74366c3a 100644 --- a/docs/plans/2026-06-26-delmia-recipe-notifier-design.md +++ b/docs/plans/2026-06-26-delmia-recipe-notifier-design.md @@ -22,7 +22,7 @@ stdout contract, same exit-code semantics — so Delmia's invocation and output | Auth | `X-API-Key: ` header | | CLI/output | Exact parity with legacy `WWNotifier` (flags + `YES`/`NO` + exit code) | | Config | `appsettings.json` next to the exe (URLs, timeout, optional log path) | -| Secret | API key from env var `SCADABRIDGE_API_KEY` — never in a file | +| Secret | API key from env var `SCADABRIDGE_API_KEY` — never in a file **(relaxed 2026-09-02: optional `ScadaBridge:ApiKey` file fallback; env still wins)** | | Packaging | Self-contained **Native AOT**, `win-x64` (fast startup for per-invocation use) | | Failover | Comma-list of base URLs; advance **only on connect failure** | | Location | New project `src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier`, in `ZB.MOM.WW.ScadaBridge.slnx` | diff --git a/env_variables.md b/env_variables.md index 2f419661..b0e6aac9 100644 --- a/env_variables.md +++ b/env_variables.md @@ -114,7 +114,7 @@ Resolution order per value: command-line flag → env var → `~/.scadabridge/co | Variable | Scope | In appsettings? | Purpose | Potential values | Required | |---|---|---|---|---|---| -| `SCADABRIDGE_API_KEY` | Runtime (client) | No — the notifier's `appsettings.json` holds only `BaseUrls`/`TimeoutSeconds`/`LogPath`; the key is **env-only by design** | Inbound API key sent as `X-API-Key`. Read only from the environment, never a file. Missing/empty → prints `API key not configured (SCADABRIDGE_API_KEY)`, exit `-1`, no HTTP attempt. | e.g. `sbk_...` | Yes | +| `SCADABRIDGE_API_KEY` | Runtime (client) | Yes — optional `ScadaBridge:ApiKey` fallback (2026-09-02); env wins | Inbound API key sent as `X-API-Key`. Environment first, file fallback (`ScadaBridge:ApiKey`, used only when the variable is unset/blank). Neither set → prints `API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)`, exit `-1`, no HTTP attempt. | e.g. `sbk_...` | Yes | > This is also why `System.Environment` is wholly forbidden in the script trust model — it would let a > site/inbound script exfiltrate `SCADABRIDGE_API_KEY` and other host env secrets. diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs index e58dae67..508b5452 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs @@ -34,12 +34,19 @@ internal static class ConfigLoader return baseUrls.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); } - /// Resolve the API key from SCADABRIDGE_API_KEY; null/whitespace → null. Env accessor is injected for testability. + /// Resolve the API key: SCADABRIDGE_API_KEY wins when non-blank, otherwise ScadaBridge:ApiKey from (trimmed); null when neither is set. Env accessor is injected for testability. /// Accessor used to read an environment variable by name. - /// The resolved API key, or null if unset/whitespace. - public static string? ResolveApiKey(Func envGet) + /// The loaded configuration, consulted only when the environment variable is unset/blank. + /// The resolved API key, or null if neither source yields a non-blank value. + public static string? ResolveApiKey(Func envGet, NotifierConfig cfg) { - var key = envGet(ApiKeyEnvVar); - return string.IsNullOrWhiteSpace(key) ? null : key; + var env = envGet(ApiKeyEnvVar); + if (!string.IsNullOrWhiteSpace(env)) + { + return env; + } + + var file = cfg.ScadaBridge.ApiKey; + return string.IsNullOrWhiteSpace(file) ? null : file.Trim(); } } diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/NotifierConfig.cs b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/NotifierConfig.cs index 196b88d9..3807f8ce 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/NotifierConfig.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/NotifierConfig.cs @@ -17,4 +17,7 @@ internal sealed class ScadaBridgeSection /// Optional diagnostic log file (relative to the exe); null/omitted → stderr only. public string? LogPath { get; set; } + + /// Optional inbound API key. SCADABRIDGE_API_KEY in the environment takes precedence; this is the file fallback (design 2026-06-26, relaxed 2026-09-02). + public string? ApiKey { get; set; } } diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs index 9d500057..f4211657 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs @@ -39,11 +39,11 @@ internal static class Program return Reporter.Report(false, "no BaseUrls configured", stdout); } - var apiKey = ConfigLoader.ResolveApiKey(Environment.GetEnvironmentVariable); + var apiKey = ConfigLoader.ResolveApiKey(Environment.GetEnvironmentVariable, cfg); if (apiKey is null) { - log.Write("API key not configured (SCADABRIDGE_API_KEY)"); - return Reporter.Report(false, "API key not configured (SCADABRIDGE_API_KEY)", stdout); + log.Write("API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)"); + return Reporter.Report(false, "API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)", stdout); } using var http = new HttpClient { Timeout = TimeSpan.FromSeconds(cfg.ScadaBridge.TimeoutSeconds) }; diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md index 6f0ea870..b61ba4f9 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md @@ -6,7 +6,7 @@ recipe / NC-program download to notify the plant that a recipe was placed at a p It is the modern, drop-in replacement for the legacy `WWNotifier` (see [`../../docs/former-api-specs/dnc/Delmia-Integration-API.md`](../../docs/former-api-specs/dnc/Delmia-Integration-API.md), *Surface B*), repointed from the old Wonderware `/notify` receiver to the **ScadaBridge Inbound API** -method `DelmiaRecipeDownload`. The command-line flags, the `YES`/`NO` stdout contract, and the +method `DelmiaRecipeDownload` (as-deployed spec: [`docs/api-specs/Delmia-Recipe-Download-API.md`](../../docs/api-specs/Delmia-Recipe-Download-API.md)). The command-line flags, the `YES`/`NO` stdout contract, and the exit-code semantics are **unchanged**, so Delmia's call site and output parsing need no changes — only the executable on disk and its `appsettings.json` are swapped. @@ -70,7 +70,8 @@ Placed next to the exe (copied to the output on build/publish): "ScadaBridge": { "BaseUrls": "http://host-a:8085,http://host-b:8085", "TimeoutSeconds": 30, - "LogPath": "logs/delmia-notifier.log" + "LogPath": "logs/delmia-notifier.log", + "ApiKey": "" } } ``` @@ -83,14 +84,16 @@ Placed next to the exe (copied to the output on build/publish): - **`TimeoutSeconds`** — per-attempt HTTP timeout (default `30`). - **`LogPath`** — optional diagnostic log file, relative to the exe. Omit to log to stderr only. -## Secret — `SCADABRIDGE_API_KEY` +## Secret — `SCADABRIDGE_API_KEY` (or `ScadaBridge:ApiKey`) -The inbound API key is read **only** from the environment variable `SCADABRIDGE_API_KEY` and is sent -as the `X-API-Key` header. It is never read from or written to a file. If unset/empty → `NO` + -`API key not configured (SCADABRIDGE_API_KEY)`, exit `-1`, no HTTP attempt. +The inbound API key is sent as `X-API-Key`. It is resolved from **`SCADABRIDGE_API_KEY`** first +(recommended for production — the key never touches disk), falling back to **`ScadaBridge:ApiKey`** in +`appsettings.json` when the variable is unset or blank. If neither is set → `NO` + +`API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)`, exit `-1`, no HTTP attempt. -Set it on the machine/account that DELMIA runs under (e.g. a system/user environment variable, or the -service account's environment). +Set the variable on the machine/account that DELMIA runs under (e.g. a system/user environment +variable, or the service account's environment). If you use the file instead, restrict its ACL to the +account DELMIA runs under — the key is stored in plain text there. ## Building & publishing (Native AOT, `win-x64`) diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/appsettings.json b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/appsettings.json index 394b41f8..20e48779 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/appsettings.json +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/appsettings.json @@ -2,6 +2,7 @@ "ScadaBridge": { "BaseUrls": "http://localhost:9000", "TimeoutSeconds": 30, - "LogPath": "logs/delmia-notifier.log" + "LogPath": "logs/delmia-notifier.log", + "ApiKey": "" } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs index 79fae0f8..87938d4a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs @@ -38,13 +38,42 @@ public class ConfigLoaderTests [Fact] public void ResolveApiKey_returns_null_when_unset_or_whitespace() { - Assert.Null(ConfigLoader.ResolveApiKey(_ => null)); - Assert.Null(ConfigLoader.ResolveApiKey(_ => " ")); + Assert.Null(ConfigLoader.ResolveApiKey(_ => null, new NotifierConfig())); + Assert.Null(ConfigLoader.ResolveApiKey(_ => " ", new NotifierConfig())); } [Fact] public void ResolveApiKey_returns_value_when_present() { - Assert.Equal("sbk_x", ConfigLoader.ResolveApiKey(n => n == "SCADABRIDGE_API_KEY" ? "sbk_x" : null)); + Assert.Equal("sbk_x", ConfigLoader.ResolveApiKey(n => n == "SCADABRIDGE_API_KEY" ? "sbk_x" : null, new NotifierConfig())); + } + + [Fact] + public void Load_reads_ApiKey_from_ScadaBridge_section() + { + var cfg = ConfigLoader.Load("{\"ScadaBridge\":{\"BaseUrls\":\"http://x\",\"ApiKey\":\"sbk_file\"}}"); + Assert.Equal("sbk_file", cfg.ScadaBridge.ApiKey); + } + + [Fact] + public void ResolveApiKey_prefers_env_over_config() + { + var cfg = ConfigLoader.Load("{\"ScadaBridge\":{\"ApiKey\":\"sbk_file\"}}"); + Assert.Equal("sbk_env", ConfigLoader.ResolveApiKey(n => n == "SCADABRIDGE_API_KEY" ? "sbk_env" : null, cfg)); + } + + [Fact] + public void ResolveApiKey_falls_back_to_config_when_env_unset_or_blank() + { + var cfg = ConfigLoader.Load("{\"ScadaBridge\":{\"ApiKey\":\" sbk_file \"}}"); + Assert.Equal("sbk_file", ConfigLoader.ResolveApiKey(_ => null, cfg)); + Assert.Equal("sbk_file", ConfigLoader.ResolveApiKey(_ => " ", cfg)); + } + + [Fact] + public void ResolveApiKey_returns_null_when_neither_source_set() + { + var cfg = ConfigLoader.Load("{\"ScadaBridge\":{\"ApiKey\":\" \"}}"); + Assert.Null(ConfigLoader.ResolveApiKey(_ => null, cfg)); } }