diff --git a/docker-env2/README.md b/docker-env2/README.md index 0caf27b0..8250c495 100644 --- a/docker-env2/README.md +++ b/docker-env2/README.md @@ -86,6 +86,9 @@ bash docker-env2/seed-data-path.sh > The MxGateway data connections need the windev gateway API key, which is not in the repo. Put it in > `~/.config/scadabridge/windev-mxgw-apikey` (mode 600), point `MXGW_APIKEY_FILE` at another file, or export > `MXGW_APIKEY`. Without a key the script skips the MxGateway connection step and seeds everything else. +> The script never puts the key on a command line: it writes the gateway config to a mode-600 temp file +> (removed on exit by a trap) and passes it with `data-connection create --primary-config-file` +> (ScadaBridge#137; see *Secrets from a file or stdin* in `src/ZB.MOM.WW.ScadaBridge.CLI/README.md`). Site-x's LocalDb is **replicated** (node-a dials node-b on 8083, same dev key on both), like the rig's site-a. Without it a site failover brings up an active node with no diff --git a/docker-env2/seed-sites.sh b/docker-env2/seed-sites.sh index 628418b2..bb646f39 100755 --- a/docker-env2/seed-sites.sh +++ b/docker-env2/seed-sites.sh @@ -55,7 +55,17 @@ if [ -z "${MXGW_APIKEY:-}" ] && [ -r "$MXGW_APIKEY_FILE" ]; then MXGW_APIKEY=$(tr -d '\r\n' < "$MXGW_APIKEY_FILE") fi MXGW_APIKEY="${MXGW_APIKEY:-}" -MXGW_CONFIG="{\"endpoint\":\"${MXGW_ENDPOINT}\",\"apiKey\":\"${MXGW_APIKEY}\",\"clientName\":\"\",\"writeUserId\":0,\"useTls\":false,\"caFile\":\"\",\"serverName\":\"\",\"readTimeoutMs\":5000}" +# The config (which carries the key) goes to the CLI through a mode-600 temp file +# (--primary-config-file), never on the command line where `ps` and shell history +# would show it (ScadaBridge#137). printf is a bash builtin, so the key is not in +# any process's argv while the file is written either. +MXGW_CONFIG_FILE="" +if [ -n "$MXGW_APIKEY" ]; then + MXGW_CONFIG_FILE=$(umask 077 && mktemp "${TMPDIR:-/tmp}/sb-mxgw-config.XXXXXX") + trap 'rm -f "$MXGW_CONFIG_FILE"' EXIT + printf '{"endpoint":"%s","apiKey":"%s","clientName":"","writeUserId":0,"useTls":false,"caFile":"","serverName":"","readTimeoutMs":5000}' \ + "$MXGW_ENDPOINT" "$MXGW_APIKEY" > "$MXGW_CONFIG_FILE" +fi SITE_X_ID=$($CLI $URL $AUTH --format json site list \ | python3 -c "import sys,json; print(next((s['id'] for s in json.load(sys.stdin) if s.get('siteIdentifier')=='site-x'), ''))" 2>/dev/null) @@ -68,7 +78,7 @@ elif [ -n "$SITE_X_ID" ]; then --site-id "$SITE_X_ID" \ --name "$CONN_NAME" \ --protocol "MxGateway" \ - --primary-config "$MXGW_CONFIG" \ + --primary-config-file "$MXGW_CONFIG_FILE" \ || echo " ('$CONN_NAME' may already exist on site-x)" else echo " (site-x not found — skipping MxGateway connection)" diff --git a/docker/README.md b/docker/README.md index 8dfa7ebc..95ceec75 100644 --- a/docker/README.md +++ b/docker/README.md @@ -185,6 +185,9 @@ docker/seed-live-probe.sh > The MxGateway data connections need the windev gateway API key, which is not in the repo. Put it in > `~/.config/scadabridge/windev-mxgw-apikey` (mode 600), point `MXGW_APIKEY_FILE` at another file, or export > `MXGW_APIKEY`. Without a key the script skips the MxGateway connection step and seeds everything else. +> The script never puts the key on a command line: it writes the gateway config to a mode-600 temp file +> (removed on exit by a trap) and passes it with `data-connection create --primary-config-file` +> (ScadaBridge#137; see *Secrets from a file or stdin* in `src/ZB.MOM.WW.ScadaBridge.CLI/README.md`). ### Live-data probe instance diff --git a/docker/seed-flow-outbound-demo.sh b/docker/seed-flow-outbound-demo.sh index 0b4fa17f..de59357f 100755 --- a/docker/seed-flow-outbound-demo.sh +++ b/docker/seed-flow-outbound-demo.sh @@ -46,7 +46,9 @@ ES_ID=$("$CLI" external-system create "${A[@]}" --name RigRest --endpoint-url ht echo " id=$ES_ID" echo "2/7 DB connection RigDb -> scadabridge-mssql/ScadaBridgeMachineData..." -DB_ID=$("$CLI" db-connection create "${A[@]}" --name RigDb --connection-string "$RIG_DB_CS" | json_id) +# The connection string carries a password: pipe it in (--connection-string-file -) so it is +# not in the CLI's argv (ScadaBridge#137). printf is a builtin, so it is not in any argv. +DB_ID=$(printf '%s' "$RIG_DB_CS" | "$CLI" db-connection create "${A[@]}" --name RigDb --connection-string-file - | json_id) echo " id=$DB_ID" echo "3/7 Notification list FlowDemo..." diff --git a/docker/seed-sites.sh b/docker/seed-sites.sh index 457ed92e..4ed1c78d 100755 --- a/docker/seed-sites.sh +++ b/docker/seed-sites.sh @@ -82,7 +82,17 @@ if [ -z "${MXGW_APIKEY:-}" ] && [ -r "$MXGW_APIKEY_FILE" ]; then MXGW_APIKEY=$(tr -d '\r\n' < "$MXGW_APIKEY_FILE") fi MXGW_APIKEY="${MXGW_APIKEY:-}" -MXGW_CONFIG="{\"endpoint\":\"${MXGW_ENDPOINT}\",\"apiKey\":\"${MXGW_APIKEY}\",\"clientName\":\"\",\"writeUserId\":0,\"useTls\":false,\"caFile\":\"\",\"serverName\":\"\",\"readTimeoutMs\":5000}" +# The config (which carries the key) goes to the CLI through a mode-600 temp file +# (--primary-config-file), never on the command line where `ps` and shell history +# would show it (ScadaBridge#137). printf is a bash builtin, so the key is not in +# any process's argv while the file is written either. +MXGW_CONFIG_FILE="" +if [ -n "$MXGW_APIKEY" ]; then + MXGW_CONFIG_FILE=$(umask 077 && mktemp "${TMPDIR:-/tmp}/sb-mxgw-config.XXXXXX") + trap 'rm -f "$MXGW_CONFIG_FILE"' EXIT + printf '{"endpoint":"%s","apiKey":"%s","clientName":"","writeUserId":0,"useTls":false,"caFile":"","serverName":"","readTimeoutMs":5000}' \ + "$MXGW_ENDPOINT" "$MXGW_APIKEY" > "$MXGW_CONFIG_FILE" +fi if [ -z "$MXGW_APIKEY" ]; then echo " (no MXGW_APIKEY / $MXGW_APIKEY_FILE — skipping MxGateway connections)" @@ -102,7 +112,7 @@ for ident in site-a site-b site-c; do --site-id "$SITE_ID" \ --name "$CONN_NAME" \ --protocol "MxGateway" \ - --primary-config "$MXGW_CONFIG" \ + --primary-config-file "$MXGW_CONFIG_FILE" \ || echo " ('$CONN_NAME' may already exist on $ident)" done fi diff --git a/docs/requirements/Component-CLI.md b/docs/requirements/Component-CLI.md index e7dcf9bb..d3471754 100644 --- a/docs/requirements/Component-CLI.md +++ b/docs/requirements/Component-CLI.md @@ -27,7 +27,7 @@ Standalone executable, not part of the Host binary. Deployed on any machine with The CLI authenticates with a bearer token exchanged once per process (review 2026-10-21 05/U1, 05/P1): -1. The user provides credentials via `--username` / `--password` (or `SCADABRIDGE_USERNAME` / `SCADABRIDGE_PASSWORD`). +1. The user provides credentials via `--username` / `--password` or `--password-file ` (or `SCADABRIDGE_USERNAME` / `SCADABRIDGE_PASSWORD`). 2. On the first management call of the process, the CLI posts the credentials to `POST /auth/token`. The server performs the LDAP bind, group lookup and role resolution, and returns a short-lived HS256 JWT carrying the username, roles and permitted sites (lifetime `JwtExpiryMinutes`, default 15 min). That is the invocation's **only** LDAP bind. 3. Every later call in the process — `POST /management`, the audit REST endpoints, and the `debug stream` hub (including its automatic reconnects) — sends `Authorization: Bearer `; the server validates it locally with no LDAP bind. A token near expiry is re-exchanged. 4. A bearer call answered `401` re-exchanges once (unless the token was minted for that very call) and, if still refused, retries with HTTP Basic and stays on Basic for the rest of the process. @@ -184,8 +184,8 @@ scadabridge deploy status [--instance-id ] [--status ] [--page ] ``` scadabridge data-connection list [--site-id ] scadabridge data-connection get --id -scadabridge data-connection create --site-id --name --protocol [--primary-config ] [--backup-config ] [--failover-retry-count ] -scadabridge data-connection update --id --name --protocol [--primary-config ] [--backup-config ] [--failover-retry-count ] +scadabridge data-connection create --site-id --name --protocol [--primary-config | --primary-config-file ] [--backup-config | --backup-config-file ] [--failover-retry-count ] +scadabridge data-connection update --id --name --protocol [--primary-config | --primary-config-file ] [--backup-config | --backup-config-file ] [--failover-retry-count ] scadabridge data-connection delete --id scadabridge data-connection move --id --site-id ``` @@ -211,8 +211,8 @@ scadabridge data-connection certs remove --site --thumbprint ``` scadabridge external-system list scadabridge external-system get --id -scadabridge external-system create --name --endpoint-url --auth-type [--auth-config ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] -scadabridge external-system update --id [--name ] [--endpoint-url ] [--auth-type ] [--auth-config ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge external-system create --name --endpoint-url --auth-type [--auth-config | --auth-config-file ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge external-system update --id [--name ] [--endpoint-url ] [--auth-type ] [--auth-config | --auth-config-file ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] scadabridge external-system delete --id scadabridge external-system method list --external-system-id scadabridge external-system method get --id @@ -234,7 +234,7 @@ scadabridge notification recipient add --list-id --name [--email scadabridge notification delete --id scadabridge notification smtp list -scadabridge notification smtp update --id --server --port --auth-mode --from-address [--tls-mode ] [--credentials ] +scadabridge notification smtp update --id --server --port --auth-mode --from-address [--tls-mode ] [--credentials | --credentials-file ] ``` Notification rename and individual recipient edits preserve unrelated IDs and @@ -242,10 +242,10 @@ names. Whole-list update replaces recipients. Recipient add requires at least one contact; the server validates it against the owning list channel. ```sh -scadabridge notification smtp create --server --port --auth-mode --from-address [--tls-mode ] [--credentials ] [--oauth2-authority ] [--oauth2-scope ] [--transport ] +scadabridge notification smtp create --server --port --auth-mode --from-address [--tls-mode ] [--credentials | --credentials-file ] [--oauth2-authority ] [--oauth2-scope ] [--transport ] scadabridge notification sms list -scadabridge notification sms create --account-sid --auth-token [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] -scadabridge notification sms update --id --account-sid [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--auth-token ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge notification sms create --account-sid (--auth-token | --auth-token-file ) [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge notification sms update --id --account-sid [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--auth-token | --auth-token-file ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] ``` SMS requires at least one sender: a From number or a Messaging Service SID. @@ -432,8 +432,8 @@ scadabridge shared-script delete --id ``` scadabridge db-connection list scadabridge db-connection get --id -scadabridge db-connection create --name --connection-string -scadabridge db-connection update --id [--name ] [--connection-string ] +scadabridge db-connection create --name (--connection-string | --connection-string-file ) +scadabridge db-connection update --id [--name ] (--connection-string | --connection-string-file ) scadabridge db-connection delete --id ``` @@ -512,15 +512,35 @@ The `--format json|table` option is recursive and accepted on every management c Configuration is resolved in the following priority order (highest wins): -1. **Command-line options**: `--url`, `--username`, `--password`, `--format`. +1. **Command-line options**: `--url`, `--username`, `--password` / `--password-file`, `--format`. 2. **Environment variables**: - `SCADABRIDGE_MANAGEMENT_URL` — Management API URL (e.g., `http://central-host:5000`). - `SCADABRIDGE_FORMAT` — Default output format (`json` or `table`). - `SCADABRIDGE_USERNAME` / `SCADABRIDGE_PASSWORD` — LDAP credentials. Preferred over `--password` on the command line, which is visible in process listings and shell history. Credentials are never read from the config file. + `--password-file ` is the per-invocation equivalent (see *Secret-bearing options*). 3. **Configuration file**: `~/.scadabridge/config.json` — Persistent defaults for management URL and output format only (never credentials). +### Secret-bearing options (file / stdin) + +Every option that can carry a secret has a `-file` sibling (ScadaBridge#137): `--password-file` +(global), `--primary-config-file` / `--backup-config-file` (`data-connection create|update`), +`--auth-config-file` (`external-system create|update`), `--connection-string-file` +(`db-connection create|update`), `--credentials-file` (`notification smtp create|update`) and +`--auth-token-file` (`notification sms create|update`). + +- `--x-file ` reads the file; `--x-file -` reads stdin (a file named `-` is `./-`). One + trailing line break is stripped. +- Inline and file forms of one value are mutually exclusive; at most one option per invocation may + read stdin; `-` on a terminal stdin is refused rather than blocking. +- The value is read at parse time. A missing, unreadable or empty source is a parse error that names + the option and path only — never the content. +- `@path` is deliberately not used: System.CommandLine expands `@`-prefixed tokens as response files. + +The inline forms remain for non-secret use. Seed scripts in `docker/` and `docker-env2/` use the +file/stdin forms for any value that carries a key or password. + ### Configuration File Format ```json diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/CliApplication.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/CliApplication.cs index 7bd134b1..b03088a6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/CliApplication.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/CliApplication.cs @@ -50,6 +50,9 @@ public static class CliApplication rootCommand.Add(SecuredWriteCommands.Build(urlOption, formatOption, usernameOption, passwordOption)); rootCommand.Add(FlowDebugCommands.Build(urlOption, formatOption, usernameOption, passwordOption)); + // After every group: the --password / --password-file exclusivity check is per command. + CliOptions.AddPasswordFileOption(rootCommand, passwordOption); + rootCommand.SetAction(_ => { Console.WriteLine("Use --help to see available commands."); diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/AuditCommandHelpers.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/AuditCommandHelpers.cs index 9ec5c1c5..3689832b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/AuditCommandHelpers.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/AuditCommandHelpers.cs @@ -88,12 +88,12 @@ public static class AuditCommandHelpers } var username = CommandHelpers.ResolveCredential(result.GetValue(usernameOption), config.Username); - var password = CommandHelpers.ResolveCredential(result.GetValue(passwordOption), config.Password); + var password = CommandHelpers.ResolvePassword(result, passwordOption, config); if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(password)) { return AuditConnection.Fail( - "Credentials required. Use --username/--password or set SCADABRIDGE_USERNAME/SCADABRIDGE_PASSWORD.", + "Credentials required. Use --username with --password or --password-file, or set SCADABRIDGE_USERNAME/SCADABRIDGE_PASSWORD.", "NO_CREDENTIALS"); } diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CliOptions.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CliOptions.cs index 90c8e906..c130fbec 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CliOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CliOptions.cs @@ -18,6 +18,40 @@ internal static class CliOptions /// than silently falling through to JSON. /// /// The configured --format option constrained to "json" or "table". + /// + /// The global --password-file option: reads the LDAP password from a file or stdin + /// (ScadaBridge#137). Shared by every root so + /// can read it without threading another option through each command group. + /// + internal static Option PasswordFileOption { get; } = + ValueSourceOptions.CreateFileOption("--password", "the LDAP password", recursive: true); + + /// + /// Adds to the root and rejects it alongside an inline + /// --password on every command. Call it after every command group is added. + /// + /// The CLI root command. + /// The inline --password option already on the root. + internal static void AddPasswordFileOption(Command root, Option passwordOption) + { + root.Add(PasswordFileOption); + + // System.CommandLine runs only the innermost command's validators, so the check goes on + // every command in the tree. Call this after all command groups are registered. + void AddTo(Command command) + { + command.Validators.Add(result => + { + if (result.GetResult(passwordOption) is { Implicit: false } && result.GetResult(PasswordFileOption) is { Implicit: false }) + result.AddError("Use either --password or --password-file, not both."); + }); + foreach (var sub in command.Subcommands) + AddTo(sub); + } + + AddTo(root); + } + internal static Option CreateFormatOption() { var formatOption = new Option("--format") diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs index 2ce0f6ae..15ac938f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/CommandHelpers.cs @@ -75,12 +75,12 @@ internal static class CommandHelpers // Resolve credentials: command-line options take precedence, then the // SCADABRIDGE_USERNAME / SCADABRIDGE_PASSWORD environment variables. var username = ResolveCredential(result.GetValue(usernameOption), config.Username); - var password = ResolveCredential(result.GetValue(passwordOption), config.Password); + var password = ResolvePassword(result, passwordOption, config); if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(password)) { OutputFormatter.WriteError( - "Credentials required. Use --username/--password or set SCADABRIDGE_USERNAME/SCADABRIDGE_PASSWORD.", + "Credentials required. Use --username with --password or --password-file, or set SCADABRIDGE_USERNAME/SCADABRIDGE_PASSWORD.", "NO_CREDENTIALS"); return 1; } @@ -145,6 +145,19 @@ internal static class CommandHelpers /// Value supplied on the command line, or null if absent. /// Fallback value from the config file or environment variable. /// The command-line value when non-empty; otherwise the environment fallback (may be null). + /// + /// Resolves the LDAP password: --password-file (file or stdin), then + /// --password, then SCADABRIDGE_PASSWORD. + /// + /// Parsed command-line result. + /// The inline --password option. + /// Loaded CLI configuration providing the environment fallback. + /// The resolved password, or null when none was supplied. + internal static string? ResolvePassword(ParseResult result, Option passwordOption, CliConfig config) + => result.GetResult(CliOptions.PasswordFileOption) is { Implicit: false } + ? result.GetValue(CliOptions.PasswordFileOption) + : ResolveCredential(result.GetValue(passwordOption), config.Password); + internal static string? ResolveCredential(string? commandLineValue, string? envValue) => string.IsNullOrWhiteSpace(commandLineValue) ? envValue : commandLineValue; diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DataConnectionCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DataConnectionCommands.cs index 0f3ff907..4a06dd19 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DataConnectionCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DataConnectionCommands.cs @@ -65,6 +65,8 @@ public static class DataConnectionCommands var protocolOption = new Option("--protocol") { Description = "Protocol", Required = true }; var configOption = new Option("--primary-config", "--configuration") { Description = "Primary configuration JSON" }; var backupConfigOption = new Option("--backup-config") { Description = "Backup configuration JSON" }; + var configFileOption = ValueSourceOptions.CreateFileOption(configOption, "the primary configuration JSON"); + var backupConfigFileOption = ValueSourceOptions.CreateFileOption(backupConfigOption, "the backup configuration JSON"); var failoverRetryOption = new Option("--failover-retry-count") { Description = "Number of retries before failover to backup", DefaultValueFactory = _ => 3 }; var cmd = new Command("update") { Description = "Update a data connection" }; @@ -74,13 +76,14 @@ public static class DataConnectionCommands cmd.Add(configOption); cmd.Add(backupConfigOption); cmd.Add(failoverRetryOption); + ValueSourceOptions.AddFileOptions(cmd, (configOption, configFileOption, false), (backupConfigOption, backupConfigFileOption, false)); cmd.SetAction(async (ParseResult result) => { var id = result.GetValue(idOption); var name = result.GetValue(nameOption)!; var protocol = result.GetValue(protocolOption)!; - var config = result.GetValue(configOption); - var backupConfig = result.GetValue(backupConfigOption); + var config = ValueSourceOptions.GetValue(result, configOption, configFileOption); + var backupConfig = ValueSourceOptions.GetValue(result, backupConfigOption, backupConfigFileOption); var failoverRetryCount = result.GetValue(failoverRetryOption); return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, @@ -110,6 +113,8 @@ public static class DataConnectionCommands var protocolOption = new Option("--protocol") { Description = "Protocol (e.g. OpcUa)", Required = true }; var configOption = new Option("--primary-config", "--configuration") { Description = "Primary configuration JSON" }; var backupConfigOption = new Option("--backup-config") { Description = "Backup configuration JSON" }; + var configFileOption = ValueSourceOptions.CreateFileOption(configOption, "the primary configuration JSON"); + var backupConfigFileOption = ValueSourceOptions.CreateFileOption(backupConfigOption, "the backup configuration JSON"); var failoverRetryOption = new Option("--failover-retry-count") { Description = "Number of retries before failover to backup", DefaultValueFactory = _ => 3 }; var cmd = new Command("create") { Description = "Create a new data connection" }; @@ -119,13 +124,14 @@ public static class DataConnectionCommands cmd.Add(configOption); cmd.Add(backupConfigOption); cmd.Add(failoverRetryOption); + ValueSourceOptions.AddFileOptions(cmd, (configOption, configFileOption, false), (backupConfigOption, backupConfigFileOption, false)); cmd.SetAction(async (ParseResult result) => { var siteId = result.GetValue(siteIdOption); var name = result.GetValue(nameOption)!; var protocol = result.GetValue(protocolOption)!; - var config = result.GetValue(configOption); - var backupConfig = result.GetValue(backupConfigOption); + var config = ValueSourceOptions.GetValue(result, configOption, configFileOption); + var backupConfig = ValueSourceOptions.GetValue(result, backupConfigOption, backupConfigFileOption); var failoverRetryCount = result.GetValue(failoverRetryOption); return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DbConnectionCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DbConnectionCommands.cs index 7dd9b14a..df2cb346 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DbConnectionCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DbConnectionCommands.cs @@ -56,15 +56,17 @@ public static class DbConnectionCommands private static Command BuildCreate(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var nameOption = new Option("--name") { Description = "Connection name", Required = true }; - var connStrOption = new Option("--connection-string") { Description = "Connection string", Required = true }; + var connStrOption = new Option("--connection-string") { Description = "Connection string (or use --connection-string-file)" }; + var connStrFileOption = ValueSourceOptions.CreateFileOption(connStrOption, "the connection string"); var cmd = new Command("create") { Description = "Create a database connection" }; cmd.Add(nameOption); cmd.Add(connStrOption); + ValueSourceOptions.AddFileOptions(cmd, (connStrOption, connStrFileOption, true)); cmd.SetAction(async (ParseResult result) => { var name = result.GetValue(nameOption)!; - var connStr = result.GetValue(connStrOption)!; + var connStr = ValueSourceOptions.GetValue(result, connStrOption, connStrFileOption)!; return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, new CreateDatabaseConnectionDefCommand(name, connStr)); @@ -76,17 +78,19 @@ public static class DbConnectionCommands { var idOption = new Option("--id") { Description = "Database connection ID", Required = true }; var nameOption = new Option("--name") { Description = "Connection name", Required = true }; - var connStrOption = new Option("--connection-string") { Description = "Connection string", Required = true }; + var connStrOption = new Option("--connection-string") { Description = "Connection string (or use --connection-string-file)" }; + var connStrFileOption = ValueSourceOptions.CreateFileOption(connStrOption, "the connection string"); var cmd = new Command("update") { Description = "Update a database connection" }; cmd.Add(idOption); cmd.Add(nameOption); cmd.Add(connStrOption); + ValueSourceOptions.AddFileOptions(cmd, (connStrOption, connStrFileOption, true)); cmd.SetAction(async (ParseResult result) => { var id = result.GetValue(idOption); var name = result.GetValue(nameOption)!; - var connStr = result.GetValue(connStrOption)!; + var connStr = ValueSourceOptions.GetValue(result, connStrOption, connStrFileOption)!; return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, new UpdateDatabaseConnectionDefCommand(id, name, connStr)); diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DebugCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DebugCommands.cs index 0f25b6d5..6c893407 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DebugCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/DebugCommands.cs @@ -71,12 +71,12 @@ public static class DebugCommands } var username = CommandHelpers.ResolveCredential(result.GetValue(usernameOption), config.Username); - var password = CommandHelpers.ResolveCredential(result.GetValue(passwordOption), config.Password); + var password = CommandHelpers.ResolvePassword(result, passwordOption, config); if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(password)) { OutputFormatter.WriteError( - "Credentials required. Use --username/--password or set SCADABRIDGE_USERNAME/SCADABRIDGE_PASSWORD.", + "Credentials required. Use --username with --password or --password-file, or set SCADABRIDGE_USERNAME/SCADABRIDGE_PASSWORD.", "NO_CREDENTIALS"); return 1; } diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ExternalSystemCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ExternalSystemCommands.cs index d3c908f8..75fcd8cb 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ExternalSystemCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ExternalSystemCommands.cs @@ -49,6 +49,7 @@ public static class ExternalSystemCommands var endpointUrlOption = new Option("--endpoint-url") { Description = "Endpoint URL", Required = true }; var authTypeOption = new Option("--auth-type") { Description = "Auth type", Required = true }; var authConfigOption = new Option("--auth-config") { Description = "Auth configuration JSON" }; + var authConfigFileOption = ValueSourceOptions.CreateFileOption(authConfigOption, "the auth configuration JSON"); var timeoutOption = new Option("--timeout-seconds") { Description = "Per-system HTTP timeout in seconds (0 = use gateway default); omit to preserve" }; var maxRetriesOption = new Option("--max-retries") { Description = "Store-and-forward retry budget for cached calls (0 = use engine default); omit to preserve" }; var retryDelayOption = new Option("--retry-delay-seconds") { Description = "Fixed interval between store-and-forward retries, in seconds (0 = use engine default); omit to preserve" }; @@ -63,13 +64,14 @@ public static class ExternalSystemCommands cmd.Add(timeoutOption); cmd.Add(maxRetriesOption); cmd.Add(retryDelayOption); + ValueSourceOptions.AddFileOptions(cmd, (authConfigOption, authConfigFileOption, false)); cmd.SetAction(async (ParseResult result) => { var id = result.GetValue(idOption); var name = result.GetValue(nameOption)!; var endpointUrl = result.GetValue(endpointUrlOption)!; var authType = result.GetValue(authTypeOption)!; - var authConfig = result.GetValue(authConfigOption); + var authConfig = ValueSourceOptions.GetValue(result, authConfigOption, authConfigFileOption); var timeoutSeconds = result.GetValue(timeoutOption); var maxRetries = result.GetValue(maxRetriesOption); var retryDelaySeconds = result.GetValue(retryDelayOption); @@ -99,6 +101,7 @@ public static class ExternalSystemCommands var endpointUrlOption = new Option("--endpoint-url") { Description = "Endpoint URL", Required = true }; var authTypeOption = new Option("--auth-type") { Description = "Auth type (ApiKey, BasicAuth)", Required = true }; var authConfigOption = new Option("--auth-config") { Description = "Auth configuration JSON" }; + var authConfigFileOption = ValueSourceOptions.CreateFileOption(authConfigOption, "the auth configuration JSON"); var timeoutOption = new Option("--timeout-seconds") { Description = "Per-system HTTP timeout in seconds (0 = use gateway default)" }; var maxRetriesOption = new Option("--max-retries") { Description = "Store-and-forward retry budget for cached calls (0 = use engine default)" }; var retryDelayOption = new Option("--retry-delay-seconds") { Description = "Fixed interval between store-and-forward retries, in seconds (0 = use engine default)" }; @@ -112,12 +115,13 @@ public static class ExternalSystemCommands cmd.Add(timeoutOption); cmd.Add(maxRetriesOption); cmd.Add(retryDelayOption); + ValueSourceOptions.AddFileOptions(cmd, (authConfigOption, authConfigFileOption, false)); cmd.SetAction(async (ParseResult result) => { var name = result.GetValue(nameOption)!; var endpointUrl = result.GetValue(endpointUrlOption)!; var authType = result.GetValue(authTypeOption)!; - var authConfig = result.GetValue(authConfigOption); + var authConfig = ValueSourceOptions.GetValue(result, authConfigOption, authConfigFileOption); var timeoutSeconds = result.GetValue(timeoutOption); var maxRetries = result.GetValue(maxRetriesOption); var retryDelaySeconds = result.GetValue(retryDelayOption); diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs index 42069215..3ceff78d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/NotificationCommands.cs @@ -182,9 +182,11 @@ public static class NotificationCommands var createCmd = new Command("create") { Description = "Create SMTP configuration" }; foreach (var option in new Option[] { SmtpServerOption, SmtpPortOption, SmtpAuthModeOption, SmtpFromOption, SmtpTlsModeOption, SmtpCredentialsOption, SmtpOAuth2AuthorityOption, SmtpOAuth2ScopeOption, SmtpTransportOption }) createCmd.Add(option); + ValueSourceOptions.AddFileOptions(createCmd, (SmtpCredentialsOption, SmtpCredentialsFileOption, false)); createCmd.SetAction(async result => await CommandHelpers.ExecuteCommandAsync(result, urlOption, formatOption, usernameOption, passwordOption, new CreateSmtpConfigCommand(result.GetValue(SmtpServerOption)!, result.GetValue(SmtpPortOption), result.GetValue(SmtpAuthModeOption)!, - result.GetValue(SmtpFromOption)!, result.GetValue(SmtpTlsModeOption), result.GetValue(SmtpCredentialsOption), + result.GetValue(SmtpFromOption)!, result.GetValue(SmtpTlsModeOption), + ValueSourceOptions.GetValue(result, SmtpCredentialsOption, SmtpCredentialsFileOption), result.GetValue(SmtpOAuth2AuthorityOption), result.GetValue(SmtpOAuth2ScopeOption), result.GetValue(SmtpTransportOption)))); group.Add(createCmd); @@ -199,6 +201,7 @@ public static class NotificationCommands updateCmd.Add(SmtpOAuth2AuthorityOption); updateCmd.Add(SmtpOAuth2ScopeOption); updateCmd.Add(SmtpTransportOption); + ValueSourceOptions.AddFileOptions(updateCmd, (SmtpCredentialsOption, SmtpCredentialsFileOption, false)); updateCmd.SetAction(async (ParseResult result) => { return await CommandHelpers.ExecuteCommandAsync( @@ -229,6 +232,9 @@ public static class NotificationCommands Description = "SMTP credentials — 'username:password' for Basic, or client secret " + "for OAuth2 (optional; preserves existing if omitted)", }; + // Declared after SmtpCredentialsOption: static initialisers run in textual order. + private static readonly Option SmtpCredentialsFileOption = + ValueSourceOptions.CreateFileOption(SmtpCredentialsOption, "the SMTP credentials"); private static readonly Option SmtpOAuth2AuthorityOption = new("--oauth2-authority") { @@ -282,7 +288,7 @@ public static class NotificationCommands var authMode = result.GetValue(SmtpAuthModeOption)!; var from = result.GetValue(SmtpFromOption)!; var tlsMode = result.GetValue(SmtpTlsModeOption); - var credentials = result.GetValue(SmtpCredentialsOption); + var credentials = ValueSourceOptions.GetValue(result, SmtpCredentialsOption, SmtpCredentialsFileOption); var oauth2Authority = result.GetValue(SmtpOAuth2AuthorityOption); var oauth2Scope = result.GetValue(SmtpOAuth2ScopeOption); var transport = result.GetValue(SmtpTransportOption); @@ -304,15 +310,18 @@ public static class NotificationCommands var createCmd = new Command("create") { Description = "Create SMS configuration (Auth Token required)" }; foreach (var option in new Option[] { SmsAccountSidOption, SmsFromNumberOption, SmsMessagingServiceSidOption, SmsApiBaseUrlOption, SmsAuthTokenOption, SmsTimeoutOption, SmsMaxRetriesOption, SmsRetryDelayOption }) createCmd.Add(option); + ValueSourceOptions.AddFileOptions(createCmd, (SmsAuthTokenOption, SmsAuthTokenFileOption, false)); createCmd.Validators.Add(result => { - if (string.IsNullOrWhiteSpace(result.GetValue(SmsAuthTokenOption))) result.AddError("--auth-token is required when creating SMS configuration."); + // A file-supplied token that failed to read already carries its own parse error. + if (result.GetResult(SmsAuthTokenFileOption) is null && string.IsNullOrWhiteSpace(result.GetValue(SmsAuthTokenOption))) + result.AddError("--auth-token (or --auth-token-file) is required when creating SMS configuration."); if (string.IsNullOrWhiteSpace(result.GetValue(SmsFromNumberOption)) && string.IsNullOrWhiteSpace(result.GetValue(SmsMessagingServiceSidOption))) result.AddError("Provide --from-number and/or --messaging-service-sid."); }); createCmd.SetAction(async result => await CommandHelpers.ExecuteCommandAsync(result, urlOption, formatOption, usernameOption, passwordOption, new CreateSmsConfigCommand(result.GetValue(SmsAccountSidOption)!, result.GetValue(SmsFromNumberOption), result.GetValue(SmsMessagingServiceSidOption), - result.GetValue(SmsApiBaseUrlOption), result.GetValue(SmsAuthTokenOption), result.GetValue(SmsTimeoutOption), + result.GetValue(SmsApiBaseUrlOption), ValueSourceOptions.GetValue(result, SmsAuthTokenOption, SmsAuthTokenFileOption), result.GetValue(SmsTimeoutOption), result.GetValue(SmsMaxRetriesOption), SmsRetryDelay(result)))); group.Add(createCmd); @@ -326,6 +335,7 @@ public static class NotificationCommands updateCmd.Add(SmsTimeoutOption); updateCmd.Add(SmsMaxRetriesOption); updateCmd.Add(SmsRetryDelayOption); + ValueSourceOptions.AddFileOptions(updateCmd, (SmsAuthTokenOption, SmsAuthTokenFileOption, false)); updateCmd.SetAction(async (ParseResult result) => { return await CommandHelpers.ExecuteCommandAsync( @@ -381,6 +391,9 @@ public static class NotificationCommands { Description = "Twilio Auth Token (optional; PRESERVES the stored token if omitted; never printed back)", }; + // Declared after SmsAuthTokenOption: static initialisers run in textual order. + private static readonly Option SmsAuthTokenFileOption = + ValueSourceOptions.CreateFileOption(SmsAuthTokenOption, "the Twilio Auth Token"); /// /// Builds the from a parsed sms update @@ -398,7 +411,7 @@ public static class NotificationCommands var fromNumber = result.GetValue(SmsFromNumberOption); var messagingServiceSid = result.GetValue(SmsMessagingServiceSidOption); var apiBaseUrl = result.GetValue(SmsApiBaseUrlOption); - var authToken = result.GetValue(SmsAuthTokenOption); + var authToken = ValueSourceOptions.GetValue(result, SmsAuthTokenOption, SmsAuthTokenFileOption); // Whole-replace update: the sender identity must be re-supplied. A Twilio config // needs a From number and/or a Messaging Service SID, so reject the case where the // update would leave neither (mirrors the UI + delivery-adapter validation). diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ValueSourceOptions.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ValueSourceOptions.cs new file mode 100644 index 00000000..27a2de8d --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/ValueSourceOptions.cs @@ -0,0 +1,187 @@ +using System.CommandLine; +using System.CommandLine.Parsing; + +namespace ZB.MOM.WW.ScadaBridge.CLI.Commands; + +/// +/// Gives a secret-bearing option a --<name>-file sibling so its value can come from a +/// file or from stdin instead of the command line, where any local user can read it from the +/// process list and where it lands in shell history (ScadaBridge#137). +/// +/// +/// Convention: --primary-config-file <path> reads the value from +/// <path>; --primary-config-file - reads it from stdin (a file literally +/// named - is reached as ./-). One trailing line break is stripped, so a file +/// written by echo works for a connection string or token. The inline option keeps +/// working; giving both is a parse error, as is reading stdin for two options at once. +/// This follows the CLI's existing --config-file / --file options rather than a +/// curl-style @path value, because System.CommandLine already treats a token that starts +/// with @ as a response file and would expand it before the option ever saw it. +/// The value is read while parsing, so a missing or unreadable file is reported as an +/// ordinary parse error. Error text names the option and the path only — never the content. +/// +internal static class ValueSourceOptions +{ + /// The value that selects stdin instead of a file path. + internal const string StdinMarker = "-"; + + /// Opens stdin for reading. Replaceable by tests. + internal static Func StdinReader { get; set; } = () => Console.In; + + /// Whether stdin is redirected (piped or from a file). Replaceable by tests. + internal static Func StdinIsRedirected { get; set; } = () => Console.IsInputRedirected; + + /// + /// Creates the --<inline>-file sibling of . + /// + /// The inline option the file option stands in for. + /// Short description of the value, used in the help text. + /// An option whose parsed value is the file (or stdin) content. + internal static Option CreateFileOption(Option inlineOption, string what) + => CreateFileOption(inlineOption.Name, what, inlineOption.Recursive); + + /// Creates the --<inline>-file option for an inline option known by name. + /// The inline option's name, including the leading dashes. + /// Short description of the value, used in the help text. + /// Whether the option applies to every subcommand. + /// An option whose parsed value is the file (or stdin) content. + internal static Option CreateFileOption(string inlineName, string what, bool recursive = false) + { + var name = inlineName + "-file"; + return new Option(name) + { + Description = $"Read {what} from a file instead of the command line ('-' reads stdin); " + + $"keeps secrets out of the process list and shell history. Replaces {inlineName}.", + Recursive = recursive, + CustomParser = argument => Read(argument, name), + }; + } + + /// + /// Adds each file option to and a validator that rejects an inline + /// value given together with its file sibling, more than one option reading stdin, and — for a + /// required pair — neither form given. + /// + /// The command that already holds the inline options. + /// Each inline option, its file sibling, and whether one of the two is required. + internal static void AddFileOptions(Command command, params (Option Inline, Option File, bool Required)[] pairs) + { + foreach (var pair in pairs) + command.Add(pair.File); + + command.Validators.Add(result => + { + var stdinUsers = new List(); + foreach (var (inline, file, required) in pairs) + { + var inlineGiven = IsGiven(result.GetResult(inline)); + var fileResult = result.GetResult(file); + var fileGiven = IsGiven(fileResult); + if (inlineGiven && fileGiven) + result.AddError($"Use either {inline.Name} or {file.Name}, not both."); + else if (required && !inlineGiven && !fileGiven) + result.AddError($"Option '{inline.Name}' (or '{file.Name}') is required."); + + if (fileGiven && ReadsStdin(fileResult!)) + stdinUsers.Add(file.Name); + } + + // The global --password-file can read stdin too. + var passwordFile = result.GetResult(CliOptions.PasswordFileOption); + if (IsGiven(passwordFile) && ReadsStdin(passwordFile!)) + stdinUsers.Add(CliOptions.PasswordFileOption.Name); + + if (stdinUsers.Count > 1) + result.AddError($"Only one option can read stdin ('-'); got {string.Join(", ", stdinUsers)}."); + }); + } + + /// + /// Returns the value from when it was given, otherwise the inline value. + /// + /// The parse result. + /// The inline option. + /// Its file sibling. + /// The resolved value, or null when neither was given. + internal static string? GetValue(ParseResult result, Option inline, Option file) + => IsGiven(result.GetResult(file)) + ? result.GetValue(file) + : result.GetResult(inline)?.GetValueOrDefault(); + + private static bool IsGiven(OptionResult? optionResult) + => optionResult is { Implicit: false }; + + private static bool ReadsStdin(OptionResult optionResult) + => optionResult.Tokens.Count == 1 && optionResult.Tokens[0].Value == StdinMarker; + + private static string? Read(ArgumentResult argument, string optionName) + { + if (argument.Tokens.Count != 1) + { + argument.AddError($"{optionName} takes exactly one path (or '-' for stdin)."); + return null; + } + + var path = argument.Tokens[0].Value; + string content; + try + { + if (path == StdinMarker) + { + if (!StdinIsRedirected()) + { + argument.AddError($"{optionName} -: stdin is a terminal; pipe or redirect the value in."); + return null; + } + + content = StdinReader().ReadToEnd(); + } + else + { + content = File.ReadAllText(path); + } + } + // Each message names the option and path only. The exception's own message is not + // used: it is short today, but the content must never be able to reach the output. + catch (FileNotFoundException) + { + argument.AddError($"{optionName}: file '{path}' not found."); + return null; + } + catch (DirectoryNotFoundException) + { + argument.AddError($"{optionName}: file '{path}' not found."); + return null; + } + catch (UnauthorizedAccessException) + { + argument.AddError($"{optionName}: file '{path}' cannot be read (access denied, or it is a directory)."); + return null; + } + catch (IOException) + { + argument.AddError($"{optionName}: file '{path}' cannot be read."); + return null; + } + + content = StripOneTrailingLineBreak(content); + if (content.Length == 0) + { + argument.AddError(path == StdinMarker + ? $"{optionName} -: stdin was empty." + : $"{optionName}: file '{path}' is empty."); + return null; + } + + return content; + } + + private static string StripOneTrailingLineBreak(string content) + { + if (content.EndsWith("\r\n", StringComparison.Ordinal)) + return content[..^2]; + if (content.EndsWith('\n')) + return content[..^1]; + return content; + } +} diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index cd3c1c40..4a9e386f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -39,6 +39,7 @@ These options are accepted by the root command and inherited by all subcommands. | `--url ` | Management API URL (e.g., `http://localhost:9001`) | | `--username ` | LDAP username for authentication | | `--password ` | LDAP password for authentication | +| `--password-file ` | Read the LDAP password from a file, or from stdin with `-` (see *Secrets from a file or stdin*). Not together with `--password`. | | `--format ` | Output format (default: `json`) | ## Configuration File @@ -60,9 +61,54 @@ For the Docker test environment, see `docker/README.md` for a ready-to-use confi | `SCADABRIDGE_MANAGEMENT_URL` | Management API URL (overrides config file) | | `SCADABRIDGE_FORMAT` | Default output format (overrides config file) | | `SCADABRIDGE_USERNAME` | LDAP username (fallback when `--username` is not supplied) | -| `SCADABRIDGE_PASSWORD` | LDAP password (fallback when `--password` is not supplied). Preferred over `--password` on the command line, which leaks into process listings and shell history. | +| `SCADABRIDGE_PASSWORD` | LDAP password (fallback when neither `--password` nor `--password-file` is supplied). Preferred over `--password` on the command line, which leaks into process listings and shell history. | | `SCADABRIDGE_AUTH_MODE` | `token` (default) exchanges credentials for a bearer token once per process; `basic` sends HTTP Basic on every call (see *Authentication* below). | +## Secrets from a file or stdin + +Every option that can carry a secret has a `-file` sibling that reads the value from a file, or from +stdin when the path is `-`, so the secret never appears on the command line. A value on the command +line can be read by any local user from the process list (`ps`) and lands in shell history. + +| Inline option | File / stdin sibling | Commands | +|---------------|----------------------|----------| +| `--password` | `--password-file` | every command (global) | +| `--primary-config` (`--configuration`) | `--primary-config-file` | `data-connection create`, `data-connection update` | +| `--backup-config` | `--backup-config-file` | `data-connection create`, `data-connection update` | +| `--auth-config` | `--auth-config-file` | `external-system create`, `external-system update` | +| `--connection-string` | `--connection-string-file` | `db-connection create`, `db-connection update` | +| `--credentials` | `--credentials-file` | `notification smtp create`, `notification smtp update` | +| `--auth-token` | `--auth-token-file` | `notification sms create`, `notification sms update` | + +Rules: + +- `--x-file ` reads the whole file; `--x-file -` reads stdin. A file literally named `-` is `./-`. +- One trailing line break is removed, so a file written with `echo` works for a connection string or token. +- The inline form keeps working for non-secret values. Giving both forms of the same value is an error, + and so is more than one option reading stdin in one invocation. +- A missing, unreadable or empty file, or `-` with stdin attached to a terminal, is a parse error that + names the option and the path. No error message ever includes the file's content. +- The value is sent to the management API exactly as the inline form would send it — the file is only + a different way to hand it to the CLI. +- `@path` is **not** a file reference: System.CommandLine treats a token that starts with `@` as a + response file and expands it before any option sees it. + +```sh +# MxGateway config with its API key, from a mode-600 file +(umask 077; printf '{"endpoint":"http://gw:5120","apiKey":"%s"}' "$MXGW_APIKEY" > /tmp/mxgw.json) +scadabridge data-connection create --site-id 1 --name "Gateway" --protocol MxGateway --primary-config-file /tmp/mxgw.json +rm -f /tmp/mxgw.json + +# Connection string from stdin +printf '%s' "$CONN_STR" | scadabridge db-connection create --name PlantDb --connection-string-file - + +# LDAP password from a file +scadabridge --password-file ~/.config/scadabridge/password site list +``` + +`printf` is a shell builtin in bash and zsh, so the secret is not in any process's argument list while +it is written either. Using `echo` or `cat <<<` is equally fine; `python3 -c ... "$SECRET"` is not. + ## Output All commands write JSON to stdout on success. Errors are written as JSON to stderr: @@ -1201,7 +1247,7 @@ scadabridge --url data-connection list [--site-id ] Create a new data connection belonging to a specific site. ```sh -scadabridge --url data-connection create --site-id --name --protocol [--primary-config ] [--backup-config ] [--failover-retry-count ] +scadabridge --url data-connection create --site-id --name --protocol [--primary-config | --primary-config-file ] [--backup-config | --backup-config-file ] [--failover-retry-count ] ``` | Option | Required | Description | @@ -1210,7 +1256,9 @@ scadabridge --url data-connection create --site-id --name - | `--name` | yes | Connection name | | `--protocol` | yes | Protocol identifier (e.g. `OpcUa`) | | `--primary-config` | no | Primary protocol-specific configuration as a JSON string (alias `--configuration`) | +| `--primary-config-file` | no | Read the primary configuration from a file, or stdin with `-` — use it when the config carries a secret (MxGateway `apiKey`, OPC UA password). Not together with `--primary-config`. | | `--backup-config` | no | Backup protocol-specific configuration as a JSON string | +| `--backup-config-file` | no | Read the backup configuration from a file, or stdin with `-`. Not together with `--backup-config`. | | `--failover-retry-count` | no | Retries before failover to the backup configuration (default: `3`) | #### `data-connection update` @@ -1219,7 +1267,7 @@ Update a data connection definition. An update **replaces** the whole entity — required field below must be supplied, even if unchanged. ```sh -scadabridge --url data-connection update --id --name --protocol [--primary-config ] [--backup-config ] [--failover-retry-count ] +scadabridge --url data-connection update --id --name --protocol [--primary-config | --primary-config-file ] [--backup-config | --backup-config-file ] [--failover-retry-count ] ``` | Option | Required | Description | @@ -1228,7 +1276,9 @@ scadabridge --url data-connection update --id --name --prot | `--name` | yes | Connection name | | `--protocol` | yes | Protocol identifier | | `--primary-config` | no | Primary protocol-specific configuration as a JSON string (alias `--configuration`) | +| `--primary-config-file` | no | Read the primary configuration from a file, or stdin with `-` — use it when the config carries a secret (MxGateway `apiKey`, OPC UA password). Not together with `--primary-config`. | | `--backup-config` | no | Backup protocol-specific configuration as a JSON string | +| `--backup-config-file` | no | Read the backup configuration from a file, or stdin with `-`. Not together with `--backup-config`. | | `--failover-retry-count` | no | Retries before failover to the backup configuration (default: `3`) | #### `data-connection move` @@ -1369,7 +1419,7 @@ scadabridge --url external-system list Register an external HTTP system that scripts can call. ```sh -scadabridge --url external-system create --name --endpoint-url --auth-type [--auth-config ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge --url external-system create --name --endpoint-url --auth-type [--auth-config | --auth-config-file ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] ``` | Option | Required | Description | @@ -1378,6 +1428,7 @@ scadabridge --url external-system create --name --endpoint-url external-system update --id --name --endpoint-url --auth-type [--auth-config ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge --url external-system update --id --name --endpoint-url --auth-type [--auth-config | --auth-config-file ] [--timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] ``` | Option | Required | Description | @@ -1402,6 +1453,7 @@ scadabridge --url external-system update --id --name --endp | `--endpoint-url` | yes | Base URL | | `--auth-type` | yes | Authentication type | | `--auth-config` | no | Updated auth credentials as a JSON string; omit to preserve the stored value | +| `--auth-config-file` | no | Read the auth configuration from a file, or stdin with `-`. Not together with `--auth-config`. | | `--timeout-seconds` | no | Per-system HTTP round-trip timeout; omit to preserve | | `--max-retries` | no | Store-and-forward retry budget for this system's `CachedCall`s; omit to preserve | | `--retry-delay-seconds` | no | Fixed interval between store-and-forward retries; omit to preserve. Alias: `--retry-delay` | @@ -1587,7 +1639,7 @@ scadabridge --url notification smtp list Create a new SMTP configuration. Requires Administrator access. ```sh -scadabridge notification smtp create --server --port --auth-mode --from-address [--tls-mode ] [--credentials ] [--oauth2-authority ] [--oauth2-scope ] [--transport ] +scadabridge notification smtp create --server --port --auth-mode --from-address [--tls-mode ] [--credentials | --credentials-file ] [--oauth2-authority ] [--oauth2-scope ] [--transport ] ``` Uses the same settings as `smtp update`, without `--id`. This sends the distinct create contract; update with ID zero is not a creation operation. The server validates transport/authentication compatibility. @@ -1597,7 +1649,7 @@ Uses the same settings as `smtp update`, without `--id`. This sends the distinct Update the SMTP configuration. ```sh -scadabridge --url notification smtp update --id --server --port --auth-mode --from-address [--tls-mode ] [--credentials ] [--oauth2-authority ] [--oauth2-scope ] [--transport ] +scadabridge --url notification smtp update --id --server --port --auth-mode --from-address [--tls-mode ] [--credentials | --credentials-file ] [--oauth2-authority ] [--oauth2-scope ] [--transport ] ``` | Option | Required | Description | @@ -1609,6 +1661,7 @@ scadabridge --url notification smtp update --id --server -- | `--from-address` | yes | Sender email address | | `--tls-mode` | no | TLS mode: `None`, `StartTLS`, or `SSL` (preserves existing if omitted) | | `--credentials` | no | SMTP credentials — `username:password` for Basic, or client secret for OAuth2 (preserves existing if omitted) | +| `--credentials-file` | no | Read the credentials from a file, or stdin with `-`. Not together with `--credentials`. | | `--oauth2-authority` | no | OAuth2 token-endpoint URL — a `{tenant}` placeholder is substituted from the credential (preserves existing / Microsoft 365 default if omitted; OAuth2 auth mode only) | | `--oauth2-scope` | no | OAuth2 scope requested from the token endpoint (preserves existing / Microsoft 365 default if omitted; OAuth2 auth mode only) | | `--transport` | no | Email transport: `Smtp` or `Ews` (preserves existing if omitted) | @@ -1630,17 +1683,17 @@ scadabridge --url notification sms list Create a new Twilio configuration. Requires Administrator access. ```sh -scadabridge notification sms create --account-sid --auth-token [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge notification sms create --account-sid (--auth-token | --auth-token-file ) [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] ``` -Account SID and auth token are required. Supply at least one sender: `--from-number` or `--messaging-service-sid`. Timeout must be positive; retries/delay must be nonnegative. Omitted policy settings use creation defaults. Explicit zero retry settings retain the outbox fallback semantics; they do not disable retries. +Account SID and auth token are required (the token inline, or from a file / stdin with `--auth-token-file`). Supply at least one sender: `--from-number` or `--messaging-service-sid`. Timeout must be positive; retries/delay must be nonnegative. Omitted policy settings use creation defaults. Explicit zero retry settings retain the outbox fallback semantics; they do not disable retries. #### `notification sms update` Update the SMS (Twilio) configuration. ```sh -scadabridge --url notification sms update --id --account-sid [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--auth-token ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] +scadabridge --url notification sms update --id --account-sid [--from-number ] [--messaging-service-sid ] [--api-base-url ] [--auth-token | --auth-token-file ] [--connection-timeout-seconds ] [--max-retries ] [--retry-delay-seconds ] ``` | Option | Required | Description | @@ -1651,6 +1704,7 @@ scadabridge --url notification sms update --id --account-sid db-connection get --id Create a new database connection definition. ```sh -scadabridge --url db-connection create --name --connection-string +scadabridge --url db-connection create --name (--connection-string | --connection-string-file ) ``` | Option | Required | Description | |--------|----------|-------------| | `--name` | yes | Connection name | -| `--connection-string` | yes | Database connection string | +| `--connection-string` | one of | Database connection string | +| `--connection-string-file` | one of | Read the connection string from a file, or stdin with `-`. Exactly one of the two is required. | #### `db-connection update` @@ -2399,14 +2454,15 @@ Update a database connection definition. An update **replaces** the whole entity every required field below must be supplied, even if unchanged. ```sh -scadabridge --url db-connection update --id --name --connection-string +scadabridge --url db-connection update --id --name (--connection-string | --connection-string-file ) ``` | Option | Required | Description | |--------|----------|-------------| | `--id` | yes | Database connection ID | | `--name` | yes | Connection name | -| `--connection-string` | yes | Database connection string | +| `--connection-string` | one of | Database connection string | +| `--connection-string-file` | one of | Read the connection string from a file, or stdin with `-`. Exactly one of the two is required. | #### `db-connection delete` diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ManagementSurfaceParityTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ManagementSurfaceParityTests.cs index f50e9dcd..53b617d9 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ManagementSurfaceParityTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ManagementSurfaceParityTests.cs @@ -185,6 +185,9 @@ public class ManagementSurfaceParityTests if (path is "notification sms create" or "notification sms update") args.AddRange(["--from-number", "+15551234567"]); if (path == "notification sms create") args.AddRange(["--auth-token", "test-token"]); + // One of --connection-string / --connection-string-file is required (validator, not Required). + if (path is "db-connection create" or "db-connection update") + args.AddRange(["--connection-string", "Server=fixture"]); if (path == "notification recipient add") args.AddRange(["--email", "recipient@example.test"]); } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ValueSourceOptionsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ValueSourceOptionsTests.cs new file mode 100644 index 00000000..3b695348 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/ValueSourceOptionsTests.cs @@ -0,0 +1,301 @@ +using System.CommandLine; +using System.Net; +using System.Net.Sockets; +using System.Text; +using System.Text.Json; +using ZB.MOM.WW.ScadaBridge.CLI.Commands; + +namespace ZB.MOM.WW.ScadaBridge.CLI.Tests; + +/// +/// ScadaBridge#137: secret-bearing options take their value from a file (--x-file path) or +/// stdin (--x-file -) so it stays off the command line. Covers the file, stdin, inline and +/// missing-file cases, and that no error ever carries the file content. +/// +[Collection("Console")] +public sealed class ValueSourceOptionsTests : IDisposable +{ + private const string Secret = "sk-137-DO-NOT-LEAK-7f3a9c"; + + private readonly string _dir = Directory.CreateTempSubdirectory("sb-cli-137-").FullName; + private readonly Func _origReader = ValueSourceOptions.StdinReader; + private readonly Func _origRedirected = ValueSourceOptions.StdinIsRedirected; + + /// Restores the stdin hooks and deletes the temp directory. + public void Dispose() + { + ValueSourceOptions.StdinReader = _origReader; + ValueSourceOptions.StdinIsRedirected = _origRedirected; + Directory.Delete(_dir, recursive: true); + } + + private string WriteFile(string content, string name = "value.txt") + { + var path = Path.Combine(_dir, name); + File.WriteAllText(path, content); + return path; + } + + private static string MxConfig => $"{{\"endpoint\":\"http://gw:5120\",\"apiKey\":\"{Secret}\"}}"; + + private void FakeStdin(string content) + { + ValueSourceOptions.StdinIsRedirected = () => true; + ValueSourceOptions.StdinReader = () => new StringReader(content); + } + + private static string AllErrors(ParseResult result) => string.Join("\n", result.Errors.Select(e => e.Message)); + + // ---- happy paths: the management payload carries the file / stdin / inline value ---- + + /// The primary config is read from a file into the payload. + [Fact] + public async Task PrimaryConfigFile_ReadsFileIntoPayload() + { + var path = WriteFile(MxConfig + "\n"); + var payload = await SendAsync($"data-connection create --site-id 1 --name C --protocol MxGateway --primary-config-file {path}"); + Assert.Equal(MxConfig, payload.GetProperty("primaryConfiguration").GetString()); + } + + /// '-' reads the value from stdin. + [Fact] + public async Task PrimaryConfigFile_Dash_ReadsStdin() + { + FakeStdin(MxConfig + "\r\n"); + var payload = await SendAsync("data-connection update --id 4 --name C --protocol MxGateway --primary-config-file -"); + Assert.Equal(MxConfig, payload.GetProperty("primaryConfiguration").GetString()); + } + + /// The inline form keeps working, and file + inline can mix across different options. + [Fact] + public async Task InlinePrimary_WithBackupFromFile_BothReachPayload() + { + var path = WriteFile(MxConfig); + var payload = await SendAsync( + $"data-connection create --site-id 1 --name C --protocol OpcUa --primary-config {{\"endpointUrl\":\"opc.tcp://a\"}} --backup-config-file {path}"); + Assert.Equal("{\"endpointUrl\":\"opc.tcp://a\"}", payload.GetProperty("primaryConfiguration").GetString()); + Assert.Equal(MxConfig, payload.GetProperty("backupConfiguration").GetString()); + } + + /// Only one trailing line break is stripped; inner whitespace is kept. + [Fact] + public async Task File_StripsExactlyOneTrailingLineBreak() + { + var path = WriteFile("Server=x;Password=" + Secret + "\n\n"); + var payload = await SendAsync($"db-connection create --name Db --connection-string-file {path}"); + Assert.Equal("Server=x;Password=" + Secret + "\n", payload.GetProperty("connectionString").GetString()); + } + + /// External-system auth config from a file. + [Fact] + public async Task AuthConfigFile_ReadsFileIntoPayload() + { + var path = WriteFile($"{{\"key\":\"{Secret}\"}}"); + var payload = await SendAsync($"external-system create --name E --endpoint-url http://e --auth-type ApiKey --auth-config-file {path}"); + Assert.Equal($"{{\"key\":\"{Secret}\"}}", payload.GetProperty("authConfiguration").GetString()); + } + + /// The SMS create requirement is satisfied by --auth-token-file. + [Fact] + public async Task SmsCreate_AuthTokenFile_SatisfiesRequirement() + { + FakeStdin(Secret); + var payload = await SendAsync("notification sms create --account-sid AC1 --from-number +15550100 --auth-token-file -"); + Assert.Equal(Secret, payload.GetProperty("authToken").GetString()); + } + + /// SMTP credentials from a file on update. + [Fact] + public async Task SmtpUpdate_CredentialsFile_ReadsFileIntoPayload() + { + var path = WriteFile("user:" + Secret); + var payload = await SendAsync( + $"notification smtp update --id 1 --server s --port 25 --auth-mode Basic --from-address a@b.c --credentials-file {path}"); + Assert.Equal("user:" + Secret, payload.GetProperty("credentials").GetString()); + } + + /// --password-file wins over the environment fallback and reaches the resolver. + [Fact] + public void PasswordFile_ResolvesPassword() + { + var path = WriteFile(Secret + "\n"); + var root = CliApplication.BuildRoot(); + var passwordOption = (Option)root.Options.Single(o => o.Name == "--password"); + var result = root.Parse($"site list --password-file {path}"); + Assert.Empty(result.Errors); + Assert.Equal(Secret, CommandHelpers.ResolvePassword(result, passwordOption, new CliConfig { Password = "from-env" })); + + var inline = root.Parse("site list --password inline"); + Assert.Equal("inline", CommandHelpers.ResolvePassword(inline, passwordOption, new CliConfig { Password = "from-env" })); + var none = root.Parse("site list"); + Assert.Equal("from-env", CommandHelpers.ResolvePassword(none, passwordOption, new CliConfig { Password = "from-env" })); + } + + // ---- error paths: clear message, never the content ---- + + /// A missing file is a parse error naming the option and path. + [Fact] + public void MissingFile_IsParseError_NamingOptionAndPath() + { + var path = Path.Combine(_dir, "nope.json"); + var result = CliApplication.BuildRoot().Parse($"data-connection create --site-id 1 --name C --protocol MxGateway --primary-config-file {path}"); + var errors = AllErrors(result); + Assert.Contains("--primary-config-file", errors); + Assert.Contains(path, errors); + Assert.Contains("not found", errors); + } + + /// A directory path is a read error, not a crash. + [Fact] + public void DirectoryPath_IsParseError() + { + var result = CliApplication.BuildRoot().Parse($"external-system create --name E --endpoint-url http://e --auth-type ApiKey --auth-config-file {_dir}"); + Assert.Contains("--auth-config-file", AllErrors(result)); + } + + /// An empty file is refused. + [Fact] + public void EmptyFile_IsParseError() + { + var path = WriteFile("\n"); + var result = CliApplication.BuildRoot().Parse($"db-connection create --name Db --connection-string-file {path}"); + Assert.Contains("is empty", AllErrors(result)); + } + + /// '-' on a terminal stdin is refused instead of blocking. + [Fact] + public void Stdin_NotRedirected_IsParseError() + { + ValueSourceOptions.StdinIsRedirected = () => false; + ValueSourceOptions.StdinReader = () => throw new InvalidOperationException("must not read a terminal"); + var result = CliApplication.BuildRoot().Parse("data-connection create --site-id 1 --name C --protocol MxGateway --primary-config-file -"); + Assert.Contains("stdin is a terminal", AllErrors(result)); + } + + /// Inline + file for the same value is refused — and the error does not echo the file. + [Fact] + public void InlineAndFile_Together_IsParseError_WithoutContent() + { + var path = WriteFile(MxConfig); + var result = CliApplication.BuildRoot().Parse( + $"data-connection create --site-id 1 --name C --protocol MxGateway --primary-config {{}} --primary-config-file {path}"); + var errors = AllErrors(result); + Assert.Contains("not both", errors); + Assert.DoesNotContain(Secret, errors); + } + + /// Two options cannot both read stdin. + [Fact] + public void TwoStdinReaders_IsParseError() + { + FakeStdin(MxConfig); + var result = CliApplication.BuildRoot().Parse( + "data-connection create --site-id 1 --name C --protocol MxGateway --primary-config-file - --password-file -"); + Assert.Contains("Only one option can read stdin", AllErrors(result)); + } + + /// --password and --password-file together are refused. + [Fact] + public void PasswordAndPasswordFile_Together_IsParseError() + { + var path = WriteFile(Secret); + var result = CliApplication.BuildRoot().Parse($"site list --password p --password-file {path}"); + Assert.Contains("--password-file", AllErrors(result)); + } + + /// The connection string stays required — either form satisfies it. + [Fact] + public void DbConnection_NeitherForm_IsRequiredError() + { + var result = CliApplication.BuildRoot().Parse("db-connection create --name Db"); + Assert.Contains("--connection-string", AllErrors(result)); + } + + /// + /// The whole invocation output (stdout + stderr, incl. the help System.CommandLine prints on a + /// parse error) never carries the file content, even when the file was read successfully and + /// another error stopped the command. + /// + [Fact] + public async Task ParseErrorOutput_NeverContainsFileContent() + { + var path = WriteFile(MxConfig); + var (exit, output) = await InvokeCapturedAsync( + $"data-connection create --name C --protocol MxGateway --primary-config-file {path}"); // --site-id missing + Assert.NotEqual(0, exit); + Assert.Contains("--site-id", output); + Assert.DoesNotContain(Secret, output); + } + + /// A file the server rejects (here: no server) does not echo the content either. + [Fact] + public async Task SendFailureOutput_NeverContainsFileContent() + { + var path = WriteFile(MxConfig); + var (_, output) = await InvokeCapturedAsync( + $"data-connection create --site-id 1 --name C --protocol MxGateway --primary-config-file {path} --url http://127.0.0.1:1 --username u --password p"); + Assert.DoesNotContain(Secret, output); + } + + // ---- helpers ---- + + private static async Task<(int Exit, string Output)> InvokeCapturedAsync(string arguments) + { + var origOut = Console.Out; + var origErr = Console.Error; + var buffer = new StringWriter(); + Console.SetOut(buffer); + Console.SetError(buffer); + try + { + var result = CliApplication.BuildRoot().Parse(arguments); + var exit = await result.InvokeAsync(new InvocationConfiguration { Output = buffer, Error = buffer }); + return (exit, buffer.ToString()); + } + finally + { + Console.SetOut(origOut); + Console.SetError(origErr); + } + } + + private static async Task SendAsync(string arguments) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + try + { + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + var server = Task.Run(async () => + { + using var client = await listener.AcceptTcpClientAsync(); + await using var stream = client.GetStream(); + using var reader = new StreamReader(stream, Encoding.UTF8, leaveOpen: true); + await reader.ReadLineAsync(); + var length = 0; + while (await reader.ReadLineAsync() is { Length: > 0 } header) + if (header.StartsWith("Content-Length:", StringComparison.OrdinalIgnoreCase)) + length = int.Parse(header.Split(':')[1]); + var buffer = new char[length]; + await reader.ReadBlockAsync(buffer, 0, length); + var response = Encoding.ASCII.GetBytes("HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 2\r\nConnection: close\r\n\r\n{}"); + await stream.WriteAsync(response); + return new string(buffer); + }); + + var args = arguments.Split(' ') + .Concat(new[] { "--url", $"http://127.0.0.1:{port}", "--username", "test", "--password", "test", "--format", "json" }) + .ToArray(); + var result = CliApplication.BuildRoot().Parse(args); + Assert.Empty(result.Errors); + Assert.Equal(0, await result.InvokeAsync()); + var body = await server.WaitAsync(TimeSpan.FromSeconds(5)); + using var doc = JsonDocument.Parse(body); + return doc.RootElement.GetProperty("payload").Clone(); + } + finally + { + listener.Stop(); + } + } +}