From f4363e0b4b5404876d2ca899bf411e152f71b147 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 2 Sep 2026 12:52:56 -0400 Subject: [PATCH] =?UTF-8?q?fix(delmia-notifier)+chore(docker):=20review=20?= =?UTF-8?q?follow-ups=20=E2=80=94=20trim=20the=20env=20key=20like=20the=20?= =?UTF-8?q?file=20key,=20log=20which=20source=20supplied=20the=20key=20(ne?= =?UTF-8?q?ver=20the=20key),=20hoist=20the=20not-configured=20message;=20R?= =?UTF-8?q?EADME=20warns=20never=20to=20commit=20a=20filled-in=20appsettin?= =?UTF-8?q?gs.json;=20design-doc=20body=20lines=20updated;=20seed-delmia-a?= =?UTF-8?q?pi.sh=20no=20longer=20hides=20CLI=20errors=20and=20notes=20key?= =?UTF-8?q?=20names=20are=20not=20unique?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_013L3XdmVii9i5sY4FChU8pB --- docker/seed-delmia-api.sh | 14 +++++---- ...026-06-26-delmia-recipe-notifier-design.md | 7 +++-- .../ConfigLoader.cs | 30 ++++++++++++++++--- .../Program.cs | 11 +++++-- .../README.md | 11 +++++-- .../ConfigLoaderTests.cs | 25 ++++++++++++++++ 6 files changed, 80 insertions(+), 18 deletions(-) diff --git a/docker/seed-delmia-api.sh b/docker/seed-delmia-api.sh index 801c96eb..f5eb23cb 100755 --- a/docker/seed-delmia-api.sh +++ b/docker/seed-delmia-api.sh @@ -4,7 +4,9 @@ set -euo pipefail # Seed the DelmiaRecipeDownload inbound method (prod's script verbatim, minus its dead `suf` # line — see docs/api-specs/Delmia-Recipe-Download-API.md §4) and one API key scoped to it. # Prints the one-time token as DELMIA_API_KEY=... — capture it; it cannot be retrieved again. -# Not idempotent (method name must be unique): delete with `api-method delete --id`. +# Not idempotent (method name must be unique): delete with `api-method delete --id`. Key display +# names are NOT unique — re-running mints another key with the same name; delete the stale one with +# `security api-key delete --key-id`. SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" @@ -12,6 +14,8 @@ CLI="$PROJECT_ROOT/src/ZB.MOM.WW.ScadaBridge.CLI/bin/Debug/net10.0/scadabridge" A=(--url "${SCADABRIDGE_URL:-http://localhost:9000}" --format=json --username "${SCADABRIDGE_USERNAME:-multi-role}" --password "${SCADABRIDGE_PASSWORD:-password}") KEY_NAME="${KEY_NAME:-delmia-notifier}" +json_id() { python3 -c 'import json,sys; print(json.load(sys.stdin)["id"])'; } + read -r -d '' METHOD_CODE <<'CS' || true try { System.Func P = k => Parameters.ContainsKey(k) ? Parameters[k] : null; @@ -31,13 +35,13 @@ try { CS echo "1/2 Creating inbound method DelmiaRecipeDownload (timeout 45 s)..." -METHOD_JSON=$("$CLI" api-method create "${A[@]}" --name DelmiaRecipeDownload --timeout 45 \ +METHOD_ID=$("$CLI" api-method create "${A[@]}" --name DelmiaRecipeDownload --timeout 45 \ --parameters '{"type":"object","properties":{"MachineCode":{"type":"string"},"DownloadPath":{"type":"string"},"WorkOrderNumber":{"type":"string"},"PartNumber":{"type":"string"},"JobStepNumber":{"type":"string"},"Username":{"type":"string"}},"required":[]}' \ --return-def '{"type":"object","properties":{"Result":{"type":"boolean"},"ResultText":{"type":"string"}},"required":["Result","ResultText"]}' \ - --script "$METHOD_CODE") -echo " $(echo "$METHOD_JSON" | python3 -c 'import json,sys; d=json.load(sys.stdin); print("id=%s name=%s" % (d["id"], d["name"]))')" + --script "$METHOD_CODE" | json_id) +echo " id=$METHOD_ID name=DelmiaRecipeDownload" echo "2/2 Creating API key '$KEY_NAME' scoped to DelmiaRecipeDownload..." -KEY_JSON=$("$CLI" security api-key create "${A[@]}" --name "$KEY_NAME" --methods DelmiaRecipeDownload 2>/dev/null) +KEY_JSON=$("$CLI" security api-key create "${A[@]}" --name "$KEY_NAME" --methods DelmiaRecipeDownload) TOKEN=$(echo "$KEY_JSON" | python3 -c 'import json,sys; print(json.load(sys.stdin)["token"])' 2>/dev/null || echo "$KEY_JSON" | grep -o 'sbk_[A-Za-z0-9_-]*' | head -1) echo "DELMIA_API_KEY=$TOKEN" 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 74366c3a..d8c284f1 100644 --- a/docs/plans/2026-06-26-delmia-recipe-notifier-design.md +++ b/docs/plans/2026-06-26-delmia-recipe-notifier-design.md @@ -78,8 +78,9 @@ Zero third-party NuGet dependencies (BCL only) to keep the Native-AOT surface mi `/api/DelmiaRecipeDownload` is appended by the app; each entry is a base URL only. - `TimeoutSeconds` — per-attempt `HttpClient.Timeout` (default 30). - `LogPath` — optional diagnostic log file (relative to the exe). Omit to log to stderr only. -- **API key** comes from `SCADABRIDGE_API_KEY`. If unset/empty → `NO` + "API key not configured", - exit `-1`, no attempt. The key is never read from or written to a file. +- **API key** comes from `SCADABRIDGE_API_KEY` first; since 2026-09-02 an optional `ScadaBridge:ApiKey` + in `appsettings.json` is the fallback when the variable is unset/blank (env wins). If neither is set → + `NO` + "API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)", exit `-1`, no attempt. ## Request / response @@ -170,7 +171,7 @@ cross-platform): - Result mapping: `Result true/false` and non-2xx → correct stdout + exit code. - Failover loop (via the HTTP seam / a fake handler): connect-failure advances; first responding node is final; `Result==false` does **not** advance; all-unreachable → `NO` + last error. -- Missing/empty `SCADABRIDGE_API_KEY` → fail-fast, no attempt. +- Missing/empty `SCADABRIDGE_API_KEY` **and** blank `ScadaBridge:ApiKey` → fail-fast, no attempt; env-over-file precedence pinned (2026-09-02). - Manual live smoke against `wonder-app-vd03` (`/api/DelmiaRecipeDownload`) from the Windows build, mirroring the earlier `curl` verification. diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs index 508b5452..5a5b9215 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/ConfigLoader.cs @@ -34,19 +34,41 @@ internal static class ConfigLoader return baseUrls.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); } - /// 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. + /// Human-readable name of the environment source, for diagnostics (never carries key material). + public const string ApiKeySourceEnvironment = "environment (SCADABRIDGE_API_KEY)"; + + /// Human-readable name of the file source, for diagnostics (never carries key material). + public const string ApiKeySourceFile = "appsettings.json (ScadaBridge:ApiKey)"; + + /// Resolve the API key: SCADABRIDGE_API_KEY wins when non-blank, otherwise ScadaBridge:ApiKey from ; both trimmed; null when neither is set. Env accessor is injected for testability. /// Accessor used to read an environment variable by name. /// 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) + public static string? ResolveApiKey(Func envGet, NotifierConfig cfg) => + ResolveApiKey(envGet, cfg, out _); + + /// Same as but also reports which source supplied the key, so a stale environment variable silently shadowing an edited file is diagnosable from the log. + /// Accessor used to read an environment variable by name. + /// The loaded configuration, consulted only when the environment variable is unset/blank. + /// , , or null when no source yielded a key. + /// The resolved API key, or null if neither source yields a non-blank value. + public static string? ResolveApiKey(Func envGet, NotifierConfig cfg, out string? source) { var env = envGet(ApiKeyEnvVar); if (!string.IsNullOrWhiteSpace(env)) { - return env; + source = ApiKeySourceEnvironment; + return env.Trim(); } var file = cfg.ScadaBridge.ApiKey; - return string.IsNullOrWhiteSpace(file) ? null : file.Trim(); + if (!string.IsNullOrWhiteSpace(file)) + { + source = ApiKeySourceFile; + return file.Trim(); + } + + source = null; + return null; } } diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs index f4211657..9ed47b96 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/Program.cs @@ -5,6 +5,9 @@ internal static class Program /// Entry point: loads config, sends the recipe-download notification with failover across the configured base URLs, and reports the legacy YES/NO stdout contract. /// Command-line arguments parsed into the recipe download payload. /// A task that resolves to the process exit code. + /// Stdout reason + log line when neither key source is set; one literal so the two cannot drift. + private const string ApiKeyNotConfigured = "API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)"; + public static async Task Main(string[] args) { var stdout = Console.Out; @@ -39,13 +42,15 @@ internal static class Program return Reporter.Report(false, "no BaseUrls configured", stdout); } - var apiKey = ConfigLoader.ResolveApiKey(Environment.GetEnvironmentVariable, cfg); + var apiKey = ConfigLoader.ResolveApiKey(Environment.GetEnvironmentVariable, cfg, out var apiKeySource); if (apiKey is null) { - 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); + log.Write(ApiKeyNotConfigured); + return Reporter.Report(false, ApiKeyNotConfigured, stdout); } + log.Write("api key source: " + apiKeySource); + using var http = new HttpClient { Timeout = TimeSpan.FromSeconds(cfg.ScadaBridge.TimeoutSeconds) }; var sender = new LoggingRecipeSender(new HttpRecipeSender(http, apiKey), log); diff --git a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md index b61ba4f9..1af9f1ea 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier/README.md @@ -88,12 +88,16 @@ Placed next to the exe (copied to the output on build/publish): 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` + +`appsettings.json` when the variable is unset or blank (a blank file value counts as unset too; both +values are trimmed). The diagnostic log records which source supplied the key — never the key itself — so +a stale environment variable silently shadowing an edited file is visible. If neither is set → `NO` + `API key not configured (SCADABRIDGE_API_KEY or ScadaBridge:ApiKey)`, exit `-1`, no HTTP attempt. 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. +account DELMIA runs under — the key is stored in plain text there. Fill `ApiKey` in **only on the deployed +copy**: the repo's `appsettings.json` is tracked and copied to the build output, so a key pasted there to +test locally is one `git add` away from being committed. It must stay `""` in source control. ## Building & publishing (Native AOT, `win-x64`) @@ -106,7 +110,8 @@ dotnet publish src/ZB.MOM.WW.ScadaBridge.DelmiaNotifier -c Release -r win-x64 # → bin/Release/net10.0/win-x64/publish/WWNotifier.exe (+ appsettings.json) ``` -Copy `WWNotifier.exe` **and** `appsettings.json` to the Delmia host, set `SCADABRIDGE_API_KEY`, and +Copy `WWNotifier.exe` **and** `appsettings.json` to the Delmia host, set `SCADABRIDGE_API_KEY` (or fill in +`ScadaBridge:ApiKey` on that copy), and point `BaseUrls` at the central node(s). > Managed `dotnet build` / `dotnet test` run cross-platform and are the development/CI gate; only the diff --git a/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs index 87938d4a..50fbff1e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DelmiaNotifier.Tests/ConfigLoaderTests.cs @@ -70,6 +70,31 @@ public class ConfigLoaderTests Assert.Equal("sbk_file", ConfigLoader.ResolveApiKey(_ => " ", cfg)); } + [Fact] + public void ResolveApiKey_trims_env_value_like_the_file_value() + { + Assert.Equal("sbk_env", ConfigLoader.ResolveApiKey(_ => " sbk_env \t", new NotifierConfig())); + } + + [Fact] + public void ResolveApiKey_reports_which_source_supplied_the_key() + { + var cfg = ConfigLoader.Load("{\"ScadaBridge\":{\"ApiKey\":\"sbk_file\"}}"); + ConfigLoader.ResolveApiKey(_ => "sbk_env", cfg, out var fromEnv); + ConfigLoader.ResolveApiKey(_ => null, cfg, out var fromFile); + ConfigLoader.ResolveApiKey(_ => null, new NotifierConfig(), out var none); + Assert.Equal(ConfigLoader.ApiKeySourceEnvironment, fromEnv); + Assert.Equal(ConfigLoader.ApiKeySourceFile, fromFile); + Assert.Null(none); + } + + [Fact] + public void Load_leaves_ApiKey_null_when_absent() + { + var cfg = ConfigLoader.Load("{\"ScadaBridge\":{\"BaseUrls\":\"http://x\"}}"); + Assert.Null(cfg.ScadaBridge.ApiKey); + } + [Fact] public void ResolveApiKey_returns_null_when_neither_source_set() {