diff --git a/plugins/host_agent/routes.py b/plugins/host_agent/routes.py index f5c2221..118f075 100644 --- a/plugins/host_agent/routes.py +++ b/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": { diff --git a/steward/ansible/bundled/host_agent/install.yml b/steward/ansible/bundled/host_agent/install.yml index ce50939..92af713 100644 --- a/steward/ansible/bundled/host_agent/install.yml +++ b/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 diff --git a/steward/ansible/bundled/host_agent/provision.yml b/steward/ansible/bundled/host_agent/provision.yml index 21b62ac..bd9bc83 100644 --- a/steward/ansible/bundled/host_agent/provision.yml +++ b/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 diff --git a/steward/ansible/executor.py b/steward/ansible/executor.py index ac8667e..4199ff3 100644 --- a/steward/ansible/executor.py +++ b/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]) diff --git a/steward/ansible/runner.py b/steward/ansible/runner.py index 6606596..0d1eacc 100644 --- a/steward/ansible/runner.py +++ b/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( diff --git a/steward/ansible/sources.py b/steward/ansible/sources.py index 9c0cc6e..b1919ca 100644 --- a/steward/ansible/sources.py +++ b/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(): diff --git a/steward/core/settings.py b/steward/core/settings.py index 8e17c75..aad7d41 100644 --- a/steward/core/settings.py +++ b/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 diff --git a/steward/settings/routes.py b/steward/settings/routes.py index 6478eb7..b0d7632 100644 --- a/steward/settings/routes.py +++ b/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() diff --git a/steward/templates/settings/notifications.html b/steward/templates/settings/notifications.html index ee8ebf9..4cbb834 100644 --- a/steward/templates/settings/notifications.html +++ b/steward/templates/settings/notifications.html @@ -56,10 +56,18 @@
+ Stored encrypted and never shown again: a webhook URL usually holds its own token.