fix(security): keep the MID password out of argv in get_mid_url.sh

.github/workflows/ci.yml

@@ -0,0 +1,21 @@
+name: CI
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+permissions:
+  contents: read
+
+jobs:
+  scripts:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+      - name: shellcheck
+        run: shellcheck scripts/get_mid_url.sh tests/get_mid_url_test.sh
+      - name: get_mid_url.sh tests
+        run: tests/get_mid_url_test.sh

CHANGELOG.md

@@ -5,7 +5,30 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Security
+- `scripts/get_mid_url.sh` no longer accepts `--password`: a password in argv is readable
+  through `ps(1)` and lands in shell history, which contradicted the script's own
+  "never in argv" guarantee. The password now comes only from `MID_INSTANCE_PASSWORD` in
+  an env file or the environment. Credentials are escaped for curl's config syntax, and
+  values containing line breaks are refused, so they cannot inject curl config lines.
+- The instance-supplied buildstamp is restricted to `[A-Za-z0-9._-]` before it is composed
+  into a download URL.
+
+### Fixed
+- `get_mid_url.sh` env-file parsing strips only one pair of matching surrounding quotes
+  (it used to delete every quote in the value) and accepts `export KEY=value`. Values are
+  otherwise literal, matching `docker run --env-file`.
+- An option given without a value now fails with a message instead of exiting silently.
+- A non-JSON or oddly shaped response (a hibernating PDI or a login page) gives a clear
+  error instead of a Python traceback; a null property value no longer crashes the parse.
+- CLAUDE.md's bump procedure said `--recipe` downloads and unpacks the recipe; it prints the
+  URL.
+
 ### Changed
+- `get_mid_url.sh` takes all three `MID_INSTANCE_*` settings from one source: the
+  `--env-file` if given, else the environment if any of them is set there, else `.env`.
+  It used to read only the env file. Settings are never mixed across sources, so a URL
+  from the environment can't be sent a password from a file.
 - **MID Server bumped from Xanadu Patch 4a (Jan 2025) to Australia Patch 2 (Apr 2026)**,
   four release families forward. Vendor recipe (`Dockerfile.unmodified`, `asset/`,
   `META-INF/`) refreshed from ServiceNow's Australia Linux container recipe.
@@ -20,6 +43,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   `!mid.env` negation.
 
 ### Added
+- `tests/get_mid_url_test.sh` (stubbed `curl`, no instance needed) and a CI workflow that
+  runs it with shellcheck.
 - `scripts/get_mid_url.sh` — derives the MID installer or container-recipe download URL
   from a live instance. There is no static "latest" link: the URL is built from the
   instance's MID buildstamp, so this reads `mid.buildstamp` (falling back to `mid.version`,

CLAUDE.md

@@ -26,7 +26,8 @@ Line numbers are for the current recipe; re-derive them after a recipe refresh.
 ## Version Bump Procedure
 
 1. `scripts/get_mid_url.sh --buildstamp` — read the instance's current MID buildstamp.
-2. `scripts/get_mid_url.sh --recipe` — download and unpack that release's vendor recipe zip.
+2. `curl -fLO "$(scripts/get_mid_url.sh --recipe)"`, then unzip it. `--recipe` only prints
+   that release's vendor recipe URL; it does not download anything.
 3. Replace `Dockerfile.unmodified`, `asset/*` and `META-INF/*` from the unpacked recipe.
 4. Re-derive `Dockerfile` from the new `Dockerfile.unmodified` by re-applying the two
    `LOCAL MOD` `FROM` lines. Take the vendor's installer URL unless ours is newer.
@@ -80,6 +81,8 @@ docker run --rm --platform=linux/amd64 -e MID_INSTANCE_URL=https://example.inval
   expansion, so one `.env` drives both it and the container. Copy it to `.mcp.json`
   (gitignored), point `command` at your checkout (here: `~/construction-mcp/mcp-servicenow`),
   and launch with `set -a; source .env; set +a; claude`.
+  Inside a session started that way, run `scripts/get_mid_url.sh --env-file .env`: the
+  script otherwise reads the exported values, and sourcing expands `$` in unquoted ones.
   Useful tools: `get_system_properties` (buildstamp), `list_records` (check `ecc_agent`).
 - The `sn-docs-harvester` skill refreshes `docs/servicenow-australia/`. Australia platform
   map: `8K~WUyyBmjUR0pLPxM8yeA`; subtrees `N~VhlzBMB0_33wUwBI_LtQ` (Containerized MID Server)

README.md

@@ -38,6 +38,7 @@ flowchart TD
 - Docker Desktop for Mac (Apple Silicon or Intel)
 - macOS Ventura (13.0) or later
 - At least 4GB of RAM available for Docker
+- `python3` and `curl` (used by `scripts/get_mid_url.sh`)
 - Basic knowledge of:
   - ServiceNow administration
   - Docker commands
@@ -86,6 +87,13 @@ Other useful flags: `--buildstamp` prints the raw buildstamp, and `--recipe` pri
 ServiceNow's Linux container recipe, which is how `Dockerfile.unmodified` and `asset/` get
 refreshed for a new family.
 
+The script reads `MID_INSTANCE_URL`, `MID_INSTANCE_USERNAME` and `MID_INSTANCE_PASSWORD` from
+one source: the `--env-file` you name, else the environment if any of them is exported,
+else `.env`. Env-file values are read literally, the way `docker run --env-file` reads them,
+so keep them unquoted and without inline comments. The script never takes the password as
+an argument, because argv shows up in `ps` and in shell history. Run its tests with
+`tests/get_mid_url_test.sh`; they stub `curl`, so no instance is needed.
+
 Keep the MID Server within one or two families of your instance. A MID Server several
 releases behind may fail to auto-upgrade and validate.
 

scripts/get_mid_url.sh

@@ -37,10 +37,13 @@ Options:
   --env-file FILE      Env file to read credentials from (default: .env)
   --instance URL       Instance URL or hostname (overrides the env file)
   --user NAME          Instance username (overrides the env file)
-  --password PASS      Instance password (overrides the env file)
   -h, --help           Show this help
 
-Credentials come from MID_INSTANCE_URL / MID_INSTANCE_USERNAME / MID_INSTANCE_PASSWORD.
+Settings come from MID_INSTANCE_URL / MID_INSTANCE_USERNAME / MID_INSTANCE_PASSWORD, all
+from one source: the --env-file if given, else the environment if any of them is set
+there, else .env. Env-file values are read literally, as docker --env-file reads them
+(an optional `export ` and one pair of surrounding quotes are removed). The password is
+never accepted as an argument: argv is visible through ps(1) and lands in shell history.
 
 Examples:
   scripts/get_mid_url.sh --check
@@ -50,41 +53,87 @@ USAGE
   exit "${1:-1}"
 }
 
-# Reads the env file without exporting it into the caller's shell.
+# Reads one value from the env file without exporting it into the caller's shell.
+# Values are literal, as docker --env-file reads them; only an optional `export ` and
+# one pair of matching surrounding quotes are removed. The last assignment wins.
 loadEnvFile() {
-  local file=$1 key=$2
+  local file=$1 key=$2 value
   [ -f "$file" ] || return 0
-  sed -n "s/^[[:space:]]*${key}=//p" "$file" | tail -1 | tr -d '"'"'"'\r'
+  value=$(sed -n "s/^[[:space:]]*\(export[[:space:]]\{1,\}\)\{0,1\}${key}=//p" "$file" | tail -1 | tr -d '\r')
+  if [ ${#value} -ge 2 ]; then
+    case "$value" in
+      \"*\"|\'*\') value=${value:1:${#value}-2} ;;
+    esac
+  fi
+  printf '%s' "$value"
+}
+
+# Reads a setting from the chosen source: an env file path, or "" for the environment.
+setting() {
+  local from=$1 key=$2
+  if [ -z "$from" ]; then
+    printf '%s' "${!key:-}"
+  else
+    loadEnvFile "$from" "$key"
+  fi
+}
+
+# Escapes a value for a double-quoted curl config string.
+curlQuote() {
+  local value=${1//\\/\\\\}
+  printf '%s' "${value//\"/\\\"}"
+}
+
+requireValue() {
+  if [ $# -lt 2 ] || [ -z "$2" ] || [ "${2#--}" != "$2" ]; then
+    errorExit "$1 needs a value."
+  fi
 }
 
 main() {
   local recipe=false check=false stampOnly=false
-  local envFile=".env" instance="" username="" password=""
+  local envFile="" instance="" username="" password from
 
   while [ $# -gt 0 ]; do
     case "$1" in
       --recipe)     recipe=true; shift ;;
       --buildstamp) stampOnly=true; shift ;;
       --check)      check=true; shift ;;
-      --env-file)   envFile=${2:-}; shift 2 ;;
-      --instance)   instance=${2:-}; shift 2 ;;
-      --user)       username=${2:-}; shift 2 ;;
-      --password)   password=${2:-}; shift 2 ;;
+      --env-file)   requireValue "$@"; envFile=$2; shift 2 ;;
+      --instance)   requireValue "$@"; instance=$2; shift 2 ;;
+      --user)       requireValue "$@"; username=$2; shift 2 ;;
+      --password|--password=*)
+        errorExit "--password is not supported: argv is visible through ps(1) and shell history.
+Set MID_INSTANCE_PASSWORD in the environment or in the env file instead." ;;
       -h|--help)    usage 0 ;;
       *)            echo "Unknown option: $1" 1>&2; usage ;;
     esac
   done
 
-  [ -n "$instance" ] || instance=$(loadEnvFile "$envFile" MID_INSTANCE_URL)
-  [ -n "$username" ] || username=$(loadEnvFile "$envFile" MID_INSTANCE_USERNAME)
-  [ -n "$password" ] || password=$(loadEnvFile "$envFile" MID_INSTANCE_PASSWORD)
+  # One source for all three settings, so a URL from one place never meets a password
+  # from another.
+  local sourceName
+  if [ -n "$envFile" ]; then
+    from=$envFile; sourceName=$envFile
+  elif [ -n "${MID_INSTANCE_URL:-}${MID_INSTANCE_USERNAME:-}${MID_INSTANCE_PASSWORD:-}" ]; then
+    from=""; sourceName="the environment"
+  else
+    from=.env; sourceName=.env
+  fi
+
+  [ -n "$instance" ] || instance=$(setting "$from" MID_INSTANCE_URL)
+  [ -n "$username" ] || username=$(setting "$from" MID_INSTANCE_USERNAME)
+  password=$(setting "$from" MID_INSTANCE_PASSWORD)
 
   if [ -z "$instance" ]; then
-    errorExit "No instance. Set MID_INSTANCE_URL in $envFile or pass --instance."
+    errorExit "No instance. Set MID_INSTANCE_URL in ${sourceName} or pass --instance."
   fi
   if [ -z "$username" ] || [ -z "$password" ]; then
-    errorExit "No credentials. Set MID_INSTANCE_USERNAME/MID_INSTANCE_PASSWORD in $envFile."
+    errorExit "No credentials. Set MID_INSTANCE_USERNAME/MID_INSTANCE_PASSWORD in ${sourceName}."
   fi
+  case "$username$password" in
+    *$'\n'*|*$'\r'*) errorExit "Credentials must not contain line breaks." ;;
+  esac
 
   # Accept https://host/, https://host, or a bare host.
   local host=${instance#*://}
@@ -95,24 +144,35 @@ main() {
 
   # Credentials go in on stdin via --config, never in argv where ps(1) could read them.
   local response
-  if ! response=$(printf 'user = "%s:%s"\n' "$username" "$password" \
+  if ! response=$(printf 'user = "%s:%s"\n' "$(curlQuote "$username")" "$(curlQuote "$password")" \
       | curl -sS -f -m 30 --config - -H "Accept: application/json" "$api" 2>&1); then
     errorExit "Could not query ${host}: ${response}
-Check the instance is awake (PDIs hibernate and are reclaimed after 10 idle days) and that the credentials in ${envFile} are valid."
+Check the instance is awake (PDIs hibernate and are reclaimed after 10 idle days) and that the credentials in ${sourceName} are valid."
   fi
 
   # mid.buildstamp is the canonical property, but some releases only populate mid.version.
   local buildstamp
-  buildstamp=$(echo "$response" | python3 -c '
+  if ! buildstamp=$(printf '%s' "$response" | python3 -c '
 import json, sys
-rows = {r["name"]: r["value"].strip() for r in json.load(sys.stdin).get("result", [])}
+try:
+    result = json.load(sys.stdin).get("result") or []
+    rows = {r.get("name"): str(r.get("value") or "").strip() for r in result if isinstance(r, dict)}
+except (ValueError, TypeError, AttributeError):
+    sys.exit(1)
 print(rows.get("mid.buildstamp") or rows.get("mid.version") or "")
-')
+'); then
+    errorExit "${host} did not return the expected JSON. A hibernating PDI or a login page answers with HTML; wake the instance and retry."
+  fi
 
   if [ -z "$buildstamp" ]; then
     errorExit "Neither mid.buildstamp nor mid.version is set on ${host}."
   fi
 
+  # The buildstamp becomes part of a URL and a docker build arg: allow only its known alphabet.
+  case "$buildstamp" in
+    *[!A-Za-z0-9._-]*) errorExit "Unexpected characters in buildstamp from ${host}: $(printf '%q' "$buildstamp")" ;;
+  esac
+
   if [ "$stampOnly" = true ]; then
     echo "$buildstamp"
     return 0