feat(secrets): webhook URL and git tokens sealed; run output masks its secrets (#5037)

#5037 "Secrets at rest and in run output: webhook URL and git token
encrypted, askpass via env, secret values scrubbed from logs".

- webhook.url joins SECRET_KEYS; existing values are sealed at startup by
  migrate_plaintext_secrets. Settings -> Notifications no longer echoes the
  URL: it names the saved host, keeps it when left blank, and has a Remove
  box.
- Git source http_token is sealed inside ansible.sources on save and at
  startup, opened in get_sources, and counted by the undecryptable-secrets
  scan as ansible.sources[<name>].http_token.
- The askpass script holds no token: git runs it with STEWARD_GIT_TOKEN in
  its environment and printf prints it as data.
- The executor masks every secret the run is handed (become/vault
  passwords, bootstrap passwords, sensitive run vars, and the per-host agent
  tokens the host_agent plugin now passes as redact) in each line before it
  is broadcast, logged or stored.
- Bundled "Write agent config" tasks are no_log; a test fails if any bundled
  task templating steward_token lacks it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

plugins/host_agent/routes.py

@@ -959,6 +959,7 @@ async def deploy_via_ansible():
         playbook_path="host_agent/install.yml",
         inventory_content=inventory_content,
         inventory_scope=scope,
+        redact=list(tokens.values()),
         # extra-vars outrank the playbook's `vars:` defaults; token stays per-host.
         params={"extra_vars_map": {"steward_url": url, "agent_interval": interval}},
         triggered_by=session.get("user_id"),
@@ -1036,6 +1037,7 @@ async def provision_via_ansible():
         playbook_path="host_agent/provision.yml",
         inventory_content=inventory_content,
         inventory_scope=scope,
+        redact=list(tokens.values()),
         # extra_vars_map → a JSON -e @file: highest precedence (beats play vars)
         # and space-safe (steward_pubkey carries a comment with a space).
         params={"extra_vars_map": {

steward/ansible/bundled/host_agent/install.yml

@@ -61,6 +61,8 @@
       notify: restart steward-agent
 
     - name: Write agent config
+      # The content carries steward_token; --diff and a failure would print it.
+      no_log: true
       ansible.builtin.copy:
         dest: /etc/steward-agent.conf
         owner: root

steward/ansible/bundled/host_agent/provision.yml

@@ -117,6 +117,8 @@
       notify: restart steward-agent
 
     - name: Write agent config
+      # The content carries steward_token; --diff and a failure would print it.
+      no_log: true
       ansible.builtin.copy:
         dest: /etc/steward-agent.conf
         owner: root

steward/ansible/executor.py

@@ -293,6 +293,45 @@ def cancel_run(run_id: str) -> bool:
 
 # ── Run execution ─────────────────────────────────────────────────────────────
 
+# ── Secret scrubbing ──────────────────────────────────────────────────────────
+# Ansible echoes values back: a failed task prints its args, -v prints vars,
+# --diff prints templated files. Each line is masked before it is broadcast,
+# written to the log artifact or stored on the run, so a secret the run was
+# handed never leaves it.
+MASK = "***"
+_MIN_SECRET_LEN = 4  # masking "on" or "1" would mangle the output and hide nothing
+
+
+def _strings(value) -> list[str]:
+    if isinstance(value, str):
+        return [value]
+    if isinstance(value, dict):
+        return [s for v in value.values() for s in _strings(v)]
+    if isinstance(value, (list, tuple, set)):
+        return [s for v in value for s in _strings(v)]
+    return []
+
+
+def run_secrets(creds: dict | None, connection: dict | None,
+                secret_vars: dict | None, redact: list[str] | None) -> list[str]:
+    """Every secret value this run is handed: the global become/vault
+    passwords, the bootstrap passwords, the sensitive run variables and any
+    the caller names (a per-host agent token)."""
+    creds, connection = creds or {}, connection or {}
+    values = [creds.get("become_password"), creds.get("vault_password"),
+              connection.get("password"), connection.get("become_password"),
+              *_strings(secret_vars), *(redact or [])]
+    found = {v for v in values if isinstance(v, str) and len(v) >= _MIN_SECRET_LEN}
+    return sorted(found, key=len, reverse=True)  # longest first: no partial masks
+
+
+def scrub(line: str, secrets: list[str]) -> str:
+    for value in secrets:
+        if value in line:
+            line = line.replace(value, MASK)
+    return line
+
+
 async def start_run(
     app: "Quart",
     run_id: str,
@@ -303,6 +342,7 @@ async def start_run(
     inventory_content: str | None = None,
     connection: dict | None = None,
     secret_vars: dict | None = None,
+    redact: list[str] | None = None,
 ) -> None:
     """Execute ansible-playbook as a subprocess and update the DB run row.
 
@@ -315,10 +355,12 @@ async def start_run(
     never persisted on the AnsibleRun row, never placed on argv, and not logged.
     secret_vars are run-time playbook variables flagged sensitive; like
     connection they reach the subprocess (merged into the extra-vars file) but
-    are never persisted.
+    are never persisted. Every value in run_secrets() — redact names more — is
+    masked in each output line before it goes anywhere.
     """
     from steward.models.ansible import AnsibleRunStatus
 
+    secrets = run_secrets(app.config.get("ANSIBLE", {}), connection, secret_vars, redact)
     _run_lines[run_id] = []
     db_output = ""
     truncated = False
@@ -432,7 +474,7 @@ async def start_run(
             assert proc.stdout is not None
 
             async for raw_line in proc.stdout:
-                line = raw_line.decode(errors="replace").rstrip("\n\r")
+                line = scrub(raw_line.decode(errors="replace").rstrip("\n\r"), secrets)
                 _broadcast(run_id, line)
                 if logf is not None:
                     logf.write(line + "\n")
@@ -478,7 +520,7 @@ async def start_run(
         final_status = "cancelled" if run_id in _cancelled else "failed"
         # Surface WHY in the run output instead of an empty "failed" (e.g. ENOSPC
         # when the temp dir can't be created — the run never reaches Ansible).
-        msg = f"[run error] {type(exc).__name__}: {exc}"
+        msg = scrub(f"[run error] {type(exc).__name__}: {exc}", secrets)
         _broadcast(run_id, msg)
         if len(failures) < _FAILURE_CAP:
             failures.append(msg[:500])

steward/ansible/runner.py

@@ -28,6 +28,7 @@ async def trigger_run(
     inventory_content: str | None = None,
     connection: dict | None = None,
     secret_vars: dict | None = None,
+    redact: list[str] | None = None,
 ):
     """Resolve inventory for the scope, persist an AnsibleRun, and launch it.
 
@@ -39,7 +40,8 @@ async def trigger_run(
     first-contact provisioning — passed to the executor but deliberately NOT
     stored on the AnsibleRun row (params), so the password never lands in the DB.
     secret_vars (sensitive run-time playbook variables) are likewise passed
-    through to the executor but never persisted.
+    through to the executor but never persisted. redact names further values
+    to mask in the run's output (a bespoke inventory's per-host tokens).
     Returns (run, source, error): on success error is None; on failure run is
     None and error is a short human-readable reason.
     """
@@ -85,7 +87,7 @@ async def trigger_run(
         executor.start_run(
             app, run_id, playbook_path, inventory_path or "",
             source["path"], params or None, inventory_content,
-            connection=connection, secret_vars=secret_vars,
+            connection=connection, secret_vars=secret_vars, redact=redact,
         )
     )
     task.add_done_callback(

steward/ansible/sources.py

@@ -108,6 +108,13 @@ def is_editable_source(source: dict) -> bool:
     return source.get("type") == "local" and source.get("name") != BUILTIN_SOURCE_NAME
 
 
+def _open_token(src: dict) -> str:
+    """The source's git token in the clear (it is stored sealed)."""
+    from steward.core.crypto import decrypt_secret
+    from steward.core.settings import source_token_key
+    return decrypt_secret(src.get("http_token") or "", context=source_token_key(src.get("name", "?")))
+
+
 def get_sources(ansible_cfg: dict) -> list[dict]:
     """Return resolved source list from ansible config section.
 
@@ -149,7 +156,7 @@ def get_sources(ansible_cfg: dict) -> list[dict]:
             "url": src.get("url"),
             "branch": src.get("branch", "main"),
             "pull_interval_seconds": int(src.get("pull_interval_seconds", 3600)),
-            "http_token": src.get("http_token", "") if src_type == "git" else "",
+            "http_token": _open_token(src) if src_type == "git" else "",
         })
     return result
 
@@ -323,11 +330,25 @@ def validate_playbook_yaml(content: str) -> tuple[bool, str | None]:
     return True, None
 
 
+# The askpass script holds no secret: git runs it with the token in its
+# environment, and printf prints it as data, so no character in a token can
+# become shell syntax.
+ASKPASS_TOKEN_ENV = "STEWARD_GIT_TOKEN"
+ASKPASS_SCRIPT = (
+    "#!/bin/sh\n"
+    'case "$1" in\n'
+    "  Username*) printf '%s\\n' oauth2 ;;\n"
+    f"  Password*) printf '%s\\n' \"${ASKPASS_TOKEN_ENV}\" ;;\n"
+    "esac\n"
+)
+
+
 async def git_pull(source: dict) -> None:
     """Clone the git repo if absent; pull if already present.
 
-    When source['http_token'] is set, injects credentials via a temporary
-    GIT_ASKPASS script so they never touch .git/config or process args.
+    When source['http_token'] is set, git asks a temporary GIT_ASKPASS script,
+    which prints the token from the child's environment, so it never touches
+    .git/config, process args or a file.
     """
     path = Path(source["path"])
     token = source.get("http_token", "")
@@ -337,18 +358,11 @@ async def git_pull(source: dict) -> None:
     if token:
         fd, askpass_path = tempfile.mkstemp(prefix="steward-askpass-", suffix=".sh")
         try:
-            script = (
-                "#!/bin/sh\n"
-                'case "$1" in\n'
-                '  Username*) echo "oauth2" ;;\n'
-                f'  Password*) echo "{token}" ;;\n'
-                "esac\n"
-            )
-            os.write(fd, script.encode())
+            os.write(fd, ASKPASS_SCRIPT.encode())
         finally:
             os.close(fd)
         os.chmod(askpass_path, stat.S_IRWXU)
-        env = {**os.environ, "GIT_ASKPASS": askpass_path}
+        env = {**os.environ, "GIT_ASKPASS": askpass_path, ASKPASS_TOKEN_ENV: token}
 
     try:
         if not (path / ".git").exists():

steward/core/settings.py

@@ -160,6 +160,9 @@ SECRET_KEYS: set[str] = {
     "ansible.ssh_private_key",
     "ansible.become_password",
     "ansible.vault_password",
+    # A webhook URL carries its own credential (Discord, Slack and ntfy put the
+    # token in the path).
+    "webhook.url",
 }
 
 
@@ -201,6 +204,31 @@ def _is_undecryptable(stored: Any, key: str = "") -> bool:
     )
 
 
+# Git sources live as a list under one setting, so their tokens are sealed
+# field by field rather than through SECRET_KEYS.
+SOURCES_KEY = "ansible.sources"
+
+
+def seal_source_tokens(sources: Any) -> tuple[Any, int]:
+    """Encrypt each source's plaintext http_token; returns (sources, how many)."""
+    from steward.core.crypto import encrypt_secret, is_encrypted
+    if not isinstance(sources, list):
+        return sources, 0
+    sealed, count = [], 0
+    for src in sources:
+        token = src.get("http_token") if isinstance(src, dict) else None
+        if isinstance(token, str) and token and not is_encrypted(token):
+            src = {**src, "http_token": encrypt_secret(token)}
+            count += 1
+        sealed.append(src)
+    return sealed, count
+
+
+def source_token_key(name: str) -> str:
+    """How a source's token is named in logs and the undecryptable-secrets banner."""
+    return f"{SOURCES_KEY}[{name}].http_token"
+
+
 # ─────────────────────────────────────────────────────────────────────────────
 # Async helpers (use inside request handlers / scheduled tasks)
 # ─────────────────────────────────────────────────────────────────────────────
@@ -507,6 +535,13 @@ def migrate_plaintext_secrets(db_url: str) -> int:
                         if isinstance(val, str) and val and not is_encrypted(val):
                             row.value_json = json.dumps(encrypt_secret(val))
                             converted += 1
+                    row = (await session.execute(
+                        select(AppSetting).where(AppSetting.key == SOURCES_KEY))).scalar_one_or_none()
+                    if row is not None:
+                        sources, sealed = seal_source_tokens(json.loads(row.value_json))
+                        if sealed:
+                            row.value_json = json.dumps(sources)
+                            converted += sealed
         finally:
             await engine.dispose()
         return converted
@@ -535,6 +570,14 @@ def scan_undecryptable_secrets(db_url: str) -> list[str]:
                 for row in result.scalars():
                     if _is_undecryptable(json.loads(row.value_json), row.key):
                         bad.append(row.key)
+                row = (await session.execute(
+                    select(AppSetting).where(AppSetting.key == SOURCES_KEY))).scalar_one_or_none()
+                sources = json.loads(row.value_json) if row is not None else []
+                for src in sources if isinstance(sources, list) else []:
+                    if isinstance(src, dict):
+                        key = source_token_key(src.get("name", "?"))
+                        if _is_undecryptable(src.get("http_token"), key):
+                            bad.append(key)
         finally:
             await engine.dispose()
         return bad

steward/settings/routes.py

@@ -11,7 +11,7 @@ from steward.core.settings import (
     get_all_settings, set_setting, delete_setting,
     get_stored_plugin_names, find_orphaned_plugins,
     to_smtp_cfg, to_webhook_cfg, to_ansible_cfg, to_plugins_cfg,
-    to_oidc_cfg, to_ldap_cfg, to_thresholds_cfg, to_access_cfg, DEFAULTS,
+    to_oidc_cfg, to_ldap_cfg, to_thresholds_cfg, to_access_cfg, DEFAULTS, seal_source_tokens,
 )
 
 settings_bp = Blueprint("settings", __name__, url_prefix="/settings")
@@ -208,7 +208,10 @@ async def save_thresholds():
 async def notifications():
     async with current_app.db_sessionmaker() as db:
         settings = await get_all_settings(db)
-    return await render_template("settings/notifications.html", settings=settings)
+    from urllib.parse import urlsplit
+    webhook_host = urlsplit(settings.get("webhook.url") or "").hostname or ""
+    return await render_template("settings/notifications.html", settings=settings,
+                                 webhook_host=webhook_host)
 
 
 @settings_bp.post("/notifications/")
@@ -226,7 +229,10 @@ async def save_notifications():
             if form.get("smtp.password"):
                 await set_setting(db, "smtp.password", form.get("smtp.password"))
             await set_setting(db, "smtp.recipients", recipients)
-            await set_setting(db, "webhook.url", form.get("webhook.url", ""))
+            if "webhook.remove" in form:
+                await set_setting(db, "webhook.url", "")
+            elif form.get("webhook.url", "").strip():
+                await set_setting(db, "webhook.url", form.get("webhook.url").strip())
             await set_setting(db, "webhook.template", form.get("webhook.template", ""))
     await _reload_app_config()
     await log_audit(current_app, session.get("user_id"), session.get("username", ""),
@@ -246,7 +252,7 @@ async def _get_ansible_sources() -> list[dict]:
 async def _save_ansible_sources(sources: list[dict]) -> None:
     async with current_app.db_sessionmaker() as db:
         async with db.begin():
-            await set_setting(db, "ansible.sources", sources)
+            await set_setting(db, "ansible.sources", seal_source_tokens(sources)[0])
     await _reload_app_config()
 
 

steward/templates/settings/notifications.html

@@ -56,10 +56,18 @@
     <h2 class="section-title" style="margin-bottom:1.25rem;">Webhook</h2>
 
     <div class="form-group">
-      <label>URL <span style="color:var(--text-muted);font-size:0.8rem;">(leave blank to disable)</span></label>
-      <input type="text" name="webhook.url" value="{{ settings['webhook.url'] }}"
-             placeholder="https://discord.com/api/webhooks/...">
+      <label>URL {% if webhook_host %}<span style="color:var(--text-muted);font-size:0.8rem;">(saved for {{ webhook_host }}; leave blank to keep it)</span>{% endif %}</label>
+      <input type="password" name="webhook.url" autocomplete="off"
+             placeholder="{% if webhook_host %}enter a new URL to replace{% else %}https://discord.com/api/webhooks/...{% endif %}">
+      <p style="font-size:0.75rem;color:var(--text-muted);margin-top:0.2rem;">
+        Stored encrypted and never shown again: a webhook URL usually holds its own token.</p>
     </div>
+    {% if webhook_host %}
+    <div class="form-group" style="display:flex;align-items:center;gap:0.5rem;">
+      <input type="checkbox" name="webhook.remove" id="webhook-remove">
+      <label for="webhook-remove" style="margin:0;">Remove the webhook</label>
+    </div>
+    {% endif %}
 
     <div class="form-group">
       <label>Payload template <span style="color:var(--text-muted);font-size:0.8rem;">(Jinja2, must produce valid JSON)</span></label>

tests/core/test_git_askpass.py

@@ -77,9 +77,9 @@ async def test_git_pull_creates_askpass_when_token_set(tmp_path):
 
 
 @pytest.mark.asyncio
-async def test_git_pull_askpass_contains_token(tmp_path):
-    """The askpass script content includes the token before cleanup."""
-    from steward.ansible.sources import git_pull
+async def test_git_pull_askpass_reads_the_token_from_its_environment(tmp_path):
+    """The askpass script holds no token; git's environment carries it."""
+    from steward.ansible.sources import ASKPASS_TOKEN_ENV, git_pull
 
     repo_path = tmp_path / "repo"
     (repo_path / ".git").mkdir(parents=True)
@@ -89,17 +89,19 @@ async def test_git_pull_askpass_contains_token(tmp_path):
         "path": str(repo_path),
         "url": "https://git.example.com/org/repo.git",
         "branch": "main",
-        "http_token": "secret-token",
+        "http_token": 'tok"$(id)`x`',
     }
 
-    script_content_captured = {}
+    captured = {}
 
     async def fake_subprocess(*args, **kwargs):
-        askpass = kwargs.get("env", {}).get("GIT_ASKPASS", "")
+        env = kwargs.get("env", {})
+        askpass = env.get("GIT_ASKPASS", "")
         if askpass and os.path.exists(askpass):
-            script_content_captured["content"] = Path(askpass).read_text()
+            captured["content"] = Path(askpass).read_text()
             mode = stat.S_IMODE(os.stat(askpass).st_mode)
-            script_content_captured["executable"] = bool(mode & stat.S_IXUSR)
+            captured["executable"] = bool(mode & stat.S_IXUSR)
+        captured["token"] = env.get(ASKPASS_TOKEN_ENV)
         proc = MagicMock()
         proc.returncode = 0
         proc.communicate = AsyncMock(return_value=(b"", b""))
@@ -108,9 +110,46 @@ async def test_git_pull_askpass_contains_token(tmp_path):
     with patch("asyncio.create_subprocess_exec", side_effect=fake_subprocess):
         await git_pull(source)
 
-    assert "secret-token" in script_content_captured.get("content", "")
-    assert "oauth2" in script_content_captured.get("content", "")
-    assert script_content_captured.get("executable") is True
+    assert "tok" not in captured["content"]
+    assert "oauth2" in captured["content"]
+    assert captured["executable"] is True
+    assert captured["token"] == 'tok"$(id)`x`'
+
+
+def test_askpass_prints_a_hostile_token_as_data(tmp_path):
+    """Run the real script: shell syntax in a token comes back verbatim."""
+    import subprocess
+    from steward.ansible.sources import ASKPASS_SCRIPT, ASKPASS_TOKEN_ENV
+
+    script = tmp_path / "askpass.sh"
+    script.write_text(ASKPASS_SCRIPT)
+    script.chmod(0o700)
+    token = 'a"b$(touch pwned)`id` c\\'
+    env = {"PATH": os.environ.get("PATH", ""), ASKPASS_TOKEN_ENV: token}
+    out = subprocess.run([str(script), "Password for 'https://x'"], env=env, cwd=tmp_path,
+                         capture_output=True, text=True, check=True).stdout
+    assert out == token + "\n"
+    assert not (tmp_path / "pwned").exists()
+    user = subprocess.run([str(script), "Username for 'https://x'"], env=env,
+                          capture_output=True, text=True, check=True).stdout
+    assert user == "oauth2\n"
+
+
+def test_sealed_tokens_open_in_get_sources(tmp_path, monkeypatch):
+    from steward.ansible.sources import get_sources
+    from steward.core import crypto
+    from steward.core.settings import seal_source_tokens
+
+    monkeypatch.setattr(crypto, "_fernet", crypto._make_fernet("unit-test-key"))
+    stored, count = seal_source_tokens([
+        {"name": "repo", "type": "git", "url": "https://x/r.git", "http_token": "mytoken123"},
+        {"name": "plain", "type": "git", "url": "https://x/p.git"},
+    ])
+    assert count == 1 and crypto.is_encrypted(stored[0]["http_token"])
+    assert seal_source_tokens(stored) == (stored, 0)  # sealing twice changes nothing
+    src = next(s for s in get_sources({"cache_dir": str(tmp_path), "sources": stored})
+               if s["name"] == "repo")
+    assert src["http_token"] == "mytoken123"
 
 
 @pytest.mark.asyncio

tests/core/test_run_scrub.py

@@ -0,0 +1,57 @@
+"""A run's secrets never reach its output: each line is masked before it is
+broadcast, written to the log artifact or stored on the run."""
+from steward.ansible.executor import MASK, run_secrets, scrub
+
+
+def test_run_secrets_gathers_everything_the_run_is_handed():
+    secrets = run_secrets(
+        {"become_password": "become-pw", "vault_password": "vault-pw", "ssh_user": "steward"},
+        {"user": "root", "password": "boot-pw"},
+        {"db": {"password": "nested-pw"}, "api_keys": ["key-one"], "port": 5432},
+        ["agent-token-abc"],
+    )
+    assert set(secrets) == {"become-pw", "vault-pw", "boot-pw", "nested-pw", "key-one",
+                            "agent-token-abc"}
+    assert "steward" not in secrets and "root" not in secrets
+
+
+def test_short_values_are_not_masked():
+    assert run_secrets(None, None, {"flag": "on", "n": "1"}, None) == []
+
+
+def test_scrub_masks_every_occurrence_longest_first():
+    secrets = run_secrets(None, None, {"a": "hunter2", "b": "hunter2-extended"}, None)
+    line = 'fatal: [h]: FAILED! => {"msg": "hunter2-extended / hunter2"}'
+    assert scrub(line, secrets) == f'fatal: [h]: FAILED! => {{"msg": "{MASK} / {MASK}"}}'
+
+
+def test_scrub_leaves_clean_lines_alone():
+    assert scrub("ok: [web1]", ["agent-token-abc"]) == "ok: [web1]"
+
+
+def test_bundled_tasks_that_template_the_agent_token_are_no_log():
+    """Every bundled task whose args mention steward_token sets no_log
+    (assert-style checks excepted: they print a fixed fail_msg)."""
+    import pathlib
+    import yaml
+    import steward.ansible
+
+    def missing(plays) -> list[str]:
+        out = []
+        for play in plays or []:
+            for task in (play.get("tasks") or []) + (play.get("handlers") or []):
+                body = {k: v for k, v in task.items() if k not in ("name", "no_log")}
+                if "ansible.builtin.assert" in body or "assert" in body:
+                    continue
+                if "steward_token" in yaml.safe_dump(body) and not task.get("no_log"):
+                    out.append(task.get("name", "?"))
+        return out
+
+    # The check can fail.
+    assert missing([{"tasks": [{"name": "leaky", "copy": {"content": "{{ steward_token }}"}}]}]) == ["leaky"]
+
+    bundled = pathlib.Path(steward.ansible.__file__).parent / "bundled"
+    files = sorted(bundled.rglob("*.yml"))
+    assert files
+    leaks = [f"{f.name}: {t}" for f in files for t in missing(yaml.safe_load(f.read_text()))]
+    assert not leaks, leaks