diff --git a/cloud_pipelines_backend/api_server_sql.py b/cloud_pipelines_backend/api_server_sql.py index 607a177..a09f8bf 100644 --- a/cloud_pipelines_backend/api_server_sql.py +++ b/cloud_pipelines_backend/api_server_sql.py @@ -32,6 +32,44 @@ def _get_current_time() -> datetime.datetime: return datetime.datetime.now(tz=datetime.timezone.utc) +_REDACTED_ON_READ: Final[str] = "" +# Lower-cased: Kubernetes uses `command`/`args`, `docker inspect` uses `Args`/`Cmd`/`Entrypoint`. +_COMMAND_LINE_FIELD_NAMES: Final[frozenset[str]] = frozenset( + {"command", "args", "cmd", "entrypoint"} +) + + +def _redact_command_lines(value: Any) -> Any: + """Returns a copy of `value` with every command line list blanked element by element. + + A container that consumes a secret as a value receives it on its command line, so the + launched object a launcher stores in `ContainerExecution.launcher_data` can hold that value + in plaintext. Launchers blank it on the way in, but a row written before they did, or by a + launcher that does not, still holds it - and at this point the stored dict is all there is, + with nothing saying which parts came from a secret. So every part goes. + + The field names cover the launcher shapes that exist: Kubernetes keeps `command` and `args` + on a container, while the Docker launcher stores a raw `docker inspect` dict, which spells + the same thing `Args`, `Config.Cmd` and `Config.Entrypoint` - hence the case-insensitive + match on a wider set of names. + + The input is never mutated: the ORM instance's JSON column must not be marked dirty by a + read request. + """ + if isinstance(value, dict): + return { + key: ( + [_REDACTED_ON_READ for _ in item] + if key.lower() in _COMMAND_LINE_FIELD_NAMES and isinstance(item, list) + else _redact_command_lines(item) + ) + for key, item in value.items() + } + if isinstance(value, list): + return [_redact_command_lines(item) for item in value] + return value + + def _get_pipeline_name_from_task_spec( *, task_spec_dict: dict[str, Any], @@ -841,7 +879,7 @@ def get_container_execution_state( exit_code=container_execution.exit_code, started_at=container_execution.started_at, ended_at=container_execution.ended_at, - debug_info=container_execution.launcher_data, + debug_info=_redact_command_lines(container_execution.launcher_data), execution_nodes_linked_to_same_container_execution=linked_execution_nodes, ) diff --git a/tests/test_execution_nodes_api_service.py b/tests/test_execution_nodes_api_service.py index 9663795..6c02e62 100644 --- a/tests/test_execution_nodes_api_service.py +++ b/tests/test_execution_nodes_api_service.py @@ -27,6 +27,9 @@ def _make_task_spec_dict(name: str = "task") -> dict: ).to_json_dict() +_PLAINTEXT_SECRET = "plaintext-secret-value" + + def _initialize_db_and_get_session_factory(): db_engine = database_ops.create_db_engine_and_migrate_db(database_uri="sqlite://") return lambda: orm.Session(bind=db_engine) @@ -441,6 +444,157 @@ def test_missing_execution_still_raises_not_found(self): session=session, id="does-not-exist" ) + def test_served_debug_info_has_no_command_line(self): + """The stored launcher data can hold a secret value on the command line. + + Which part of a command line came from a secret cannot be told from the stored dict, + so the response blanks all of them, at every depth and for every launcher shape. + """ + launcher_data = { + "kubernetes_job": { + "job_name": "task-abc", + "debug_job": { + "spec": { + "template": { + "spec": { + "containers": [ + { + "name": "main", + "image": "alpine", + "command": ["sh", "-c"], + "args": ["echo", _PLAINTEXT_SECRET], + } + ] + } + } + } + }, + "debug_pods": { + "0": { + "spec": { + "containers": [ + { + "name": "main", + "args": ["echo", _PLAINTEXT_SECRET], + } + ] + } + } + }, + } + } + with self.session_factory() as session: + node = _make_execution_node(task_spec=_make_task_spec_dict()) + node.container_execution = bts.ContainerExecution( + status=bts.ContainerExecutionStatus.SUCCEEDED, + launcher_data=launcher_data, + ) + session.add(node) + session.flush() + + result = self.service.get_container_execution_state( + session=session, id=node.id + ) + + assert result.debug_info is not None + assert _PLAINTEXT_SECRET not in str(result.debug_info) + job_container = result.debug_info["kubernetes_job"]["debug_job"]["spec"][ + "template" + ]["spec"]["containers"][0] + assert job_container["command"] == [ + "", + "", + ] + assert job_container["args"] == [ + "", + "", + ] + # Everything that is not a command line is served unchanged. + assert job_container["image"] == "alpine" + assert result.debug_info["kubernetes_job"]["job_name"] == "task-abc" + # The per-Pod copies a Job carries are blanked too. + pod_container = result.debug_info["kubernetes_job"]["debug_pods"]["0"][ + "spec" + ]["containers"][0] + assert pod_container["args"] == [ + "", + "", + ] + + def test_served_debug_info_covers_the_docker_inspect_shape(self): + """The Docker launcher stores a raw `docker inspect` dict, which capitalises the keys. + + Its entrypoint and command come straight from the resolved command line, so a secret + passed by value lands in `Args`, `Config.Cmd` and `Config.Entrypoint`. + """ + launcher_data = { + "docker": { + "id": "abc123", + "debug_container": { + "Path": "python", + "Args": ["-c", "print(1)", _PLAINTEXT_SECRET], + "Config": { + "Image": "python:3.12-slim", + "Entrypoint": ["python", "-c"], + "Cmd": ["print(1)", _PLAINTEXT_SECRET], + }, + }, + } + } + with self.session_factory() as session: + node = _make_execution_node(task_spec=_make_task_spec_dict()) + node.container_execution = bts.ContainerExecution( + status=bts.ContainerExecutionStatus.SUCCEEDED, + launcher_data=launcher_data, + ) + session.add(node) + session.flush() + + result = self.service.get_container_execution_state( + session=session, id=node.id + ) + + assert result.debug_info is not None + assert _PLAINTEXT_SECRET not in str(result.debug_info) + container = result.debug_info["docker"]["debug_container"] + assert container["Args"] == [""] * 3 + assert container["Config"]["Cmd"] == [""] * 2 + assert container["Config"]["Entrypoint"] == [""] * 2 + # Not a command line, and not a list: served unchanged. + assert container["Path"] == "python" + assert container["Config"]["Image"] == "python:3.12-slim" + + def test_serving_does_not_modify_the_stored_row(self): + """Redaction happens on a copy; a read request must not write to the JSON column.""" + launcher_data = { + "kubernetes": { + "debug_pod": { + "spec": { + "containers": [ + {"name": "main", "args": ["echo", _PLAINTEXT_SECRET]} + ] + } + } + } + } + with self.session_factory() as session: + node = _make_execution_node(task_spec=_make_task_spec_dict()) + container_execution = bts.ContainerExecution( + status=bts.ContainerExecutionStatus.SUCCEEDED, + launcher_data=launcher_data, + ) + node.container_execution = container_execution + session.add(node) + session.flush() + + self.service.get_container_execution_state(session=session, id=node.id) + + stored_args = container_execution.launcher_data["kubernetes"]["debug_pod"][ + "spec" + ]["containers"][0]["args"] + assert stored_args == ["echo", _PLAINTEXT_SECRET] + assert not session.dirty + if __name__ == "__main__": pytest.main()