From 1c12336a0f6baa483f72d7ea4e56b8d5ef3cdaaa Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 21 Jul 2026 12:24:14 +0100 Subject: [PATCH 1/4] fix(tasks): initialize sandbox credentials before agent launch --- .../backend/logic/services/docker_sandbox.py | 7 ++- .../backend/logic/services/local_packages.py | 31 +++++++++++++ .../backend/logic/services/modal_sandbox.py | 5 ++- .../services/tests/test_local_packages.py | 19 ++++++++ .../services/tests/test_modal_sandbox.py | 4 +- .../activities/provision_sandbox.py | 43 ++++++++++--------- .../test_inject_fresh_tokens_on_resume.py | 16 +++++-- .../process_task/sandbox_credentials.py | 17 ++++++++ .../tests/test_sandbox_credentials.py | 24 +++++++++++ products/tasks/backend/tests/test_agentsh.py | 7 ++- 10 files changed, 137 insertions(+), 36 deletions(-) diff --git a/products/tasks/backend/logic/services/docker_sandbox.py b/products/tasks/backend/logic/services/docker_sandbox.py index e33ab3035573..bfa7e2aaec04 100644 --- a/products/tasks/backend/logic/services/docker_sandbox.py +++ b/products/tasks/backend/logic/services/docker_sandbox.py @@ -852,16 +852,15 @@ def _build_agent_server_command( 'export NO_PROXY="host.docker.internal,${NO_PROXY:-localhost,127.0.0.1}"; export no_proxy="$NO_PROXY"; ' ) inner = f"cd /scripts && {no_proxy_export}{server_cmd} > /tmp/agent-server.log 2>&1" + initialize_env_file = f"(test -f {ENV_FILE} || env -0 > {ENV_FILE})" if allowed_domains is not None: return ( - f"cd /scripts && env -0 > {ENV_FILE} && " + f"cd /scripts && {initialize_env_file} && " f"{build_exec_prefix()} {ENV_WRAPPER_SCRIPT} bash -c {shlex.quote(inner)} &" ) else: - # Write the env file even without agentsh so BASH_ENV (and the - # in-process token resolver) can re-read a backend-refreshed token. - return f"cd /scripts && env -0 > {ENV_FILE} && nohup {server_cmd} > /tmp/agent-server.log 2>&1 &" + return f"cd /scripts && {initialize_env_file} && nohup {server_cmd} > /tmp/agent-server.log 2>&1 &" def _launch_and_check(self, command: str) -> bool: """Execute the agent-server command and wait for the health check. diff --git a/products/tasks/backend/logic/services/local_packages.py b/products/tasks/backend/logic/services/local_packages.py index dcfdc6a5c055..d3bc083509c9 100644 --- a/products/tasks/backend/logic/services/local_packages.py +++ b/products/tasks/backend/logic/services/local_packages.py @@ -18,6 +18,8 @@ from django.conf import settings +import yaml + logger = logging.getLogger(__name__) BUILD_OUTPUT_SUBDIR = "dist" @@ -81,6 +83,7 @@ def get_local_posthog_code_packages() -> tuple[LocalPackage, ...] | None: def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) -> dict[str, dict[str, str]]: """Collect registry dependencies needed by the overlaid local package builds.""" dependencies: dict[str, dict[str, str]] = {} + dependency_catalogs: dict[Path, dict[str, str]] = {} for package in packages: manifest_path = package.source_path / "package.json" @@ -95,6 +98,34 @@ def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) - raise ValueError(f"Expected dependency names and versions to be strings in {manifest_path}") if version.startswith("workspace:"): continue + if version == "catalog:": + workspace_manifest_path = next( + ( + parent / "pnpm-workspace.yaml" + for parent in package.source_path.parents + if (parent / "pnpm-workspace.yaml").is_file() + ), + None, + ) + if workspace_manifest_path is None: + raise ValueError(f"Could not resolve catalog dependency {name} from {manifest_path}") + + if workspace_manifest_path not in dependency_catalogs: + workspace_manifest = yaml.safe_load(workspace_manifest_path.read_text()) + catalog = workspace_manifest.get("catalog") if isinstance(workspace_manifest, dict) else None + if not isinstance(catalog, dict) or not all( + isinstance(catalog_name, str) and isinstance(catalog_version, str) + for catalog_name, catalog_version in catalog.items() + ): + raise ValueError( + f"Expected catalog to contain string dependencies in {workspace_manifest_path}" + ) + dependency_catalogs[workspace_manifest_path] = catalog + + resolved_version = dependency_catalogs[workspace_manifest_path].get(name) + if resolved_version is None: + raise ValueError(f"Catalog dependency {name} is missing from {workspace_manifest_path}") + version = resolved_version runtime_dependencies[name] = version if runtime_dependencies: diff --git a/products/tasks/backend/logic/services/modal_sandbox.py b/products/tasks/backend/logic/services/modal_sandbox.py index 73dc8767c185..4dc93922f282 100644 --- a/products/tasks/backend/logic/services/modal_sandbox.py +++ b/products/tasks/backend/logic/services/modal_sandbox.py @@ -1000,14 +1000,15 @@ def _build_agent_server_command( server_cmd = f"bash -c {shlex.quote(wait_for_repo)}" inner = f"cd /scripts && {server_cmd} > /tmp/agent-server.log 2>&1" + initialize_env_file = f"(test -f {ENV_FILE} || env -0 > {ENV_FILE})" if allowed_domains is not None: return ( - f"cd /scripts && env -0 > {ENV_FILE} && " + f"cd /scripts && {initialize_env_file} && " f"{build_exec_prefix()} {ENV_WRAPPER_SCRIPT} bash -c {shlex.quote(inner)} &" ) else: - return f"cd /scripts && env -0 > {ENV_FILE} && nohup {server_cmd} > /tmp/agent-server.log 2>&1 &" + return f"cd /scripts && {initialize_env_file} && nohup {server_cmd} > /tmp/agent-server.log 2>&1 &" def _diagnose_startup_failure(self, allowed_domains: list[str] | None) -> dict[str, str]: diagnostics: dict[str, str] = {} diff --git a/products/tasks/backend/logic/services/tests/test_local_packages.py b/products/tasks/backend/logic/services/tests/test_local_packages.py index 809769c41e0c..a7d0eb9f8a45 100644 --- a/products/tasks/backend/logic/services/tests/test_local_packages.py +++ b/products/tasks/backend/logic/services/tests/test_local_packages.py @@ -8,6 +8,8 @@ from products.tasks.backend.logic.services.local_packages import ( BUILD_OUTPUT_SUBDIR, PACKAGE_NAMES, + LocalPackage, + get_local_package_runtime_dependencies, get_local_posthog_code_packages, ) @@ -67,3 +69,20 @@ def test_returns_packages_when_everything_present(self, fake_monorepo: Path) -> assert packages[0].sandbox_install_path == "/scripts/node_modules/@posthog/agent" assert packages[0].sandbox_build_output_path == "/scripts/node_modules/@posthog/agent/dist" assert packages[0].build_output_path == fake_monorepo / "packages" / "agent" / "dist" + + +def test_resolves_default_catalog_runtime_dependencies(fake_monorepo: Path) -> None: + (fake_monorepo / "pnpm-workspace.yaml").write_text("catalog:\n catalog-runtime: 1.2.3\n") + agent_source_path = fake_monorepo / "packages" / "agent" + (agent_source_path / "package.json").write_text( + '{"dependencies":{"catalog-runtime":"catalog:","registry-runtime":"^4.5.6"}}' + ) + package = LocalPackage( + name="agent", + source_path=agent_source_path, + sandbox_install_path="/scripts/node_modules/@posthog/agent", + ) + + assert get_local_package_runtime_dependencies((package,)) == { + "agent": {"catalog-runtime": "1.2.3", "registry-runtime": "^4.5.6"} + } diff --git a/products/tasks/backend/logic/services/tests/test_modal_sandbox.py b/products/tasks/backend/logic/services/tests/test_modal_sandbox.py index 78ad1c937925..213dada6ded4 100644 --- a/products/tasks/backend/logic/services/tests/test_modal_sandbox.py +++ b/products/tasks/backend/logic/services/tests/test_modal_sandbox.py @@ -487,7 +487,7 @@ def test_start_agent_server_wraps_with_agentsh_when_domains_provided(self, mock_ command = _agent_server_launch_command(mock_sandbox.execute) assert "--createPr true" in command assert "agentsh exec --client-timeout 2h --timeout 2h" in command - assert "env -0 > /tmp/agent-env" in command + assert "(test -f /tmp/agent-env || env -0 > /tmp/agent-env)" in command assert "/tmp/agentsh-env-wrapper.sh" in command assert "./node_modules/.bin/agent-server" in command @@ -509,7 +509,7 @@ def test_start_agent_server_wraps_with_agentsh_when_domains_empty(self, mock_san command = _agent_server_launch_command(mock_sandbox.execute) assert "--allowedDomains" not in command assert "agentsh exec --client-timeout 2h --timeout 2h" in command - assert "env -0 > /tmp/agent-env" in command + assert "(test -f /tmp/agent-env || env -0 > /tmp/agent-env)" in command @pytest.mark.parametrize( ("create_pr", "expected_flag"), diff --git a/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py b/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py index c152f098e93d..3d67383f254d 100644 --- a/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py +++ b/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py @@ -10,8 +10,13 @@ from posthog.temporal.common.utils import asyncify from products.tasks.backend.constants import SNAPSHOT_KIND_FILESYSTEM, filter_user_sandbox_env_vars -from products.tasks.backend.exceptions import GitHubAuthenticationError, OAuthTokenError, TaskNotFoundError -from products.tasks.backend.logic.services.agentsh import ENV_FILE, INFRASTRUCTURE_DOMAINS, _get_debug_only_domains +from products.tasks.backend.exceptions import ( + GitHubAuthenticationError, + OAuthTokenError, + SandboxProvisionError, + TaskNotFoundError, +) +from products.tasks.backend.logic.services.agentsh import INFRASTRUCTURE_DOMAINS, _get_debug_only_domains from products.tasks.backend.logic.services.connection_token import ( SANDBOX_JWT_STATE_KID_KEY, get_primary_sandbox_jwt_kid, @@ -27,7 +32,11 @@ ) from products.tasks.backend.temporal.oauth import create_oauth_access_token_for_run, create_wizard_oauth_access_token from products.tasks.backend.temporal.observability import emit_agent_log, log_activity_execution -from products.tasks.backend.temporal.process_task.sandbox_credentials import set_git_remote_token +from products.tasks.backend.temporal.process_task.sandbox_credentials import ( + initialize_sandbox_env_file, + set_git_remote_token, + update_sandbox_env_file, +) from products.tasks.backend.temporal.process_task.utils import ( get_git_identity_env_vars, get_readonly_github_token, @@ -585,6 +594,14 @@ def create_sandbox_for_repository(input: CreateSandboxForRepositoryInput) -> Cre increment_sandbox_created("vm" if use_vm_sandbox else "gvisor") + if not initialize_sandbox_env_file(sandbox, prepared.environment_variables): + sandbox.destroy() + raise SandboxProvisionError( + "Failed to initialize sandbox environment", + {"task_id": ctx.task_id, "run_id": ctx.run_id, "sandbox_id": sandbox.id}, + cause=RuntimeError("sandbox environment file initialization failed"), + ) + credentials = sandbox.get_connect_credentials() try: @@ -759,11 +776,8 @@ def inject_fresh_tokens_on_resume(input: InjectFreshTokensOnResumeInput) -> None if github_token and input.repository: set_git_remote_token(sandbox, input.repository, github_token) - # Pre-seed the agentsh env file so any wrapped command that runs between - # resume and start_agent_server (diagnostics, branch checkout) sees the - # fresh tokens instead of the stale snapshot values. start_agent_server - # re-dumps the full process env over this, so a partial overwrite is fine - # here (unlike the mid-run refresh, which must preserve the live env). + # Update the initialized env file before any resumed command can observe + # credentials persisted in the snapshot. fresh_env_vars: dict[str, str] = {} if github_token: fresh_env_vars["GITHUB_TOKEN"] = github_token @@ -771,18 +785,7 @@ def inject_fresh_tokens_on_resume(input: InjectFreshTokensOnResumeInput) -> None if access_token: fresh_env_vars["POSTHOG_PERSONAL_API_KEY"] = access_token - if fresh_env_vars: - env_payload = b"".join(f"{k}={v}\x00".encode() for k, v in fresh_env_vars.items()) - overwrite_result = sandbox.write_file(ENV_FILE, env_payload) - if overwrite_result.exit_code != 0: - logger.warning( - "Failed to refresh agentsh env file on resume", - extra={ - "sandbox_id": input.sandbox_id, - "env_file": ENV_FILE, - "stderr": overwrite_result.stderr, - }, - ) + update_sandbox_env_file(sandbox, fresh_env_vars) emit_agent_log(ctx.run_id, "debug", "Refreshed sandbox credentials after resume") diff --git a/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py b/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py index b9295001bba9..83677e338a8b 100644 --- a/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py +++ b/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py @@ -1,3 +1,5 @@ +import base64 + import pytest from unittest.mock import MagicMock, patch @@ -28,6 +30,11 @@ def sandbox(self): return fake def test_refreshes_git_remote_url_and_env_file(self, activity_environment, task_context, test_task, sandbox): + existing = b"PATH=/usr/bin\x00GITHUB_TOKEN=ghs_old\x00" + sandbox.execute.side_effect = [ + ExecutionResult(stdout="", stderr="", exit_code=0), + ExecutionResult(stdout=base64.b64encode(existing).decode(), stderr="", exit_code=0), + ] with ( patch( "products.tasks.backend.temporal.process_task.activities.provision_sandbox.Sandbox.get_by_id", @@ -51,8 +58,8 @@ def test_refreshes_git_remote_url_and_env_file(self, activity_environment, task_ ), ) - assert sandbox.execute.call_count == 1 - remote_command = sandbox.execute.call_args[0][0] + assert sandbox.execute.call_count == 2 + remote_command = sandbox.execute.call_args_list[0].args[0] assert "git remote set-url origin" in remote_command assert "x-access-token:ghs_new" in remote_command assert task_context.repository in remote_command @@ -66,6 +73,7 @@ def test_refreshes_git_remote_url_and_env_file(self, activity_environment, task_ assert "GITHUB_TOKEN=ghs_new" in decoded assert "GH_TOKEN=ghs_new" in decoded assert "POSTHOG_PERSONAL_API_KEY=oauth_new" in decoded + assert "PATH=/usr/bin" in decoded def test_skips_git_remote_when_github_integration_missing(self, activity_environment, test_task, sandbox): context = TaskProcessingContext( @@ -98,7 +106,7 @@ def test_skips_git_remote_when_github_integration_missing(self, activity_environ ), ) - sandbox.execute.assert_not_called() + assert sandbox.execute.call_count == 1 # OAuth env is still written so POSTHOG_PERSONAL_API_KEY refreshes. assert sandbox.write_file.call_count == 1 _, payload = sandbox.write_file.call_args[0] @@ -133,7 +141,7 @@ def test_logs_warning_when_remote_url_update_fails(self, activity_environment, t ), ) - assert sandbox.execute.call_count == 1 + assert sandbox.execute.call_count == 2 assert sandbox.write_file.call_count == 1 def test_prepare_sandbox_injects_user_github_token_without_repository(self, activity_environment, team, user): diff --git a/products/tasks/backend/temporal/process_task/sandbox_credentials.py b/products/tasks/backend/temporal/process_task/sandbox_credentials.py index 1b37fbcd18ba..bee5f243e314 100644 --- a/products/tasks/backend/temporal/process_task/sandbox_credentials.py +++ b/products/tasks/backend/temporal/process_task/sandbox_credentials.py @@ -118,6 +118,23 @@ def update_sandbox_env_file(sandbox: "SandboxBase", updates: dict[str, str]) -> return True +def initialize_sandbox_env_file(sandbox: "SandboxBase", environment_variables: dict[str, str]) -> bool: + """Capture the sandbox environment, then overlay the values resolved for this run. + + This runs synchronously during provisioning so the agent cannot start before its + per-command shells have the same credentials as the agent-server process. + """ + capture = sandbox.execute(f"env -0 > {shlex.quote(ENV_FILE)}", timeout_seconds=30) + if capture.exit_code != 0: + logger.warning( + "Failed to initialize sandbox env file", + extra={"sandbox_id": sandbox.id, "env_file": ENV_FILE, "stderr": capture.stderr}, + ) + return False + + return update_sandbox_env_file(sandbox, environment_variables) + + def apply_github_credentials_to_sandbox(sandbox: "SandboxBase", repository: str | None, github_token: str) -> None: """Re-inject a GitHub token into both places a running sandbox reads it from.""" if repository: diff --git a/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py b/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py index 014fd645ff43..d38e5aa18ba9 100644 --- a/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py +++ b/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py @@ -10,6 +10,7 @@ GitHubSandboxCredential, build_sandbox_credentials, github_refresh_interval_seconds, + initialize_sandbox_env_file, set_git_remote_token, update_sandbox_env_file, ) @@ -102,6 +103,29 @@ def test_writes_updates_when_env_file_absent(self): assert payload == b"GH_TOKEN=ghs_new\x00" +class TestInitializeSandboxEnvFile: + def test_captures_container_environment_then_overlays_resolved_credentials(self): + sandbox = MagicMock() + existing = b"PATH=/usr/bin\x00GITHUB_TOKEN=stale\x00" + sandbox.execute.side_effect = [_ok(), _ok(base64.b64encode(existing).decode())] + sandbox.write_file.return_value = _ok() + + assert ( + initialize_sandbox_env_file( + sandbox, + {"GITHUB_TOKEN": "ghu_fresh", "GH_TOKEN": "ghu_fresh"}, + ) + is True + ) + + assert sandbox.execute.call_args_list[0].args[0] == "env -0 > /tmp/agent-env" + _, payload = sandbox.write_file.call_args[0] + entries = {entry.split(b"=", 1)[0]: entry.split(b"=", 1)[1] for entry in payload.split(b"\x00") if entry} + assert entries[b"PATH"] == b"/usr/bin" + assert entries[b"GITHUB_TOKEN"] == b"ghu_fresh" + assert entries[b"GH_TOKEN"] == b"ghu_fresh" + + class TestGitHubSandboxCredential: def test_resolves_and_applies_token_and_reports_interval(self): sandbox = MagicMock() diff --git a/products/tasks/backend/tests/test_agentsh.py b/products/tasks/backend/tests/test_agentsh.py index fb8423b08d05..bf3b805e3dc5 100644 --- a/products/tasks/backend/tests/test_agentsh.py +++ b/products/tasks/backend/tests/test_agentsh.py @@ -285,9 +285,8 @@ def test_command_without_domains_skips_agentsh_exec(self): create_pr=True, ) self.assertNotIn("agentsh exec --client-timeout 2h --timeout 2h", cmd) - # The env file is written at launch regardless of agentsh so the - # mid-session credential refresh can re-source the token per command. - self.assertIn("env -0 > /tmp/agent-env", cmd) + # Provisioning initializes the env file synchronously; legacy callers only fill a missing file. + self.assertIn("(test -f /tmp/agent-env || env -0 > /tmp/agent-env)", cmd) self.assertNotIn(ENV_WRAPPER_SCRIPT, cmd) self.assertIn("nohup", cmd) @@ -388,7 +387,7 @@ def test_command_includes_allowed_domains(self): allowed_domains=["example.com", "api.example.com"], ) self.assertIn("agentsh exec --client-timeout 2h --timeout 2h", cmd) - self.assertIn("env -0 > /tmp/agent-env", cmd) + self.assertIn("(test -f /tmp/agent-env || env -0 > /tmp/agent-env)", cmd) self.assertIn(ENV_WRAPPER_SCRIPT, cmd) self.assertIn("--allowedDomains", cmd) self.assertIn("example.com,api.example.com", cmd) From 56a1077f947f8d9bd882daa99e81cfe69a0d829f Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 21 Jul 2026 13:41:46 +0100 Subject: [PATCH 2/4] fix(tasks): preserve refreshed credentials during agent launch --- .../tasks/backend/logic/services/agentsh.py | 21 ++++++- .../backend/logic/services/docker_sandbox.py | 3 +- .../backend/logic/services/modal_sandbox.py | 3 +- .../services/tests/test_modal_sandbox.py | 4 +- .../activities/provision_sandbox.py | 16 +---- .../process_task/sandbox_credentials.py | 17 ------ .../tests/test_sandbox_credentials.py | 24 -------- products/tasks/backend/tests/test_agentsh.py | 58 +++++++++++++++++-- 8 files changed, 77 insertions(+), 69 deletions(-) diff --git a/products/tasks/backend/logic/services/agentsh.py b/products/tasks/backend/logic/services/agentsh.py index ffe0a626ec64..ea426309259a 100644 --- a/products/tasks/backend/logic/services/agentsh.py +++ b/products/tasks/backend/logic/services/agentsh.py @@ -113,16 +113,31 @@ def generate_env_wrapper() -> str: """ -def generate_bash_env_script() -> str: +def generate_bash_env_script(env_file: str = ENV_FILE) -> str: """ - Generate the script sourced via ``BASH_ENV``. + Generate the script sourced via ``BASH_ENV`` and used to initialize its env file. + + The explicit invocation runs inside the background agent-server launch. It + atomically captures the current sandbox environment only when the env file does + not already exist, so it cannot overwrite credentials refreshed by the backend. + Sourced invocations stay cheap and only export the GitHub credentials needed by + tool shells. """ + quoted_env_file = shlex.quote(env_file) return f"""\ +if [[ "${{BASH_SOURCE[0]}}" == "$0" ]]; then + tmp_file={quoted_env_file}.tmp.$$ + trap 'rm -f "$tmp_file"' EXIT + env -0 > "$tmp_file" + ln "$tmp_file" {quoted_env_file} 2>/dev/null || true + exit 0 +fi + while IFS= read -r -d $'\\0' kv 2>/dev/null; do case "$kv" in GH_TOKEN=*|GITHUB_TOKEN=*) export "$kv" ;; esac -done < {ENV_FILE} 2>/dev/null +done < {quoted_env_file} 2>/dev/null """ diff --git a/products/tasks/backend/logic/services/docker_sandbox.py b/products/tasks/backend/logic/services/docker_sandbox.py index bfa7e2aaec04..299b69cfe506 100644 --- a/products/tasks/backend/logic/services/docker_sandbox.py +++ b/products/tasks/backend/logic/services/docker_sandbox.py @@ -34,7 +34,6 @@ from .agentsh import ( BASH_ENV_SCRIPT, - ENV_FILE, ENV_WRAPPER_SCRIPT, SESSION_ID_FILE, build_exec_prefix, @@ -852,7 +851,7 @@ def _build_agent_server_command( 'export NO_PROXY="host.docker.internal,${NO_PROXY:-localhost,127.0.0.1}"; export no_proxy="$NO_PROXY"; ' ) inner = f"cd /scripts && {no_proxy_export}{server_cmd} > /tmp/agent-server.log 2>&1" - initialize_env_file = f"(test -f {ENV_FILE} || env -0 > {ENV_FILE})" + initialize_env_file = f"bash {shlex.quote(BASH_ENV_SCRIPT)}" if allowed_domains is not None: return ( diff --git a/products/tasks/backend/logic/services/modal_sandbox.py b/products/tasks/backend/logic/services/modal_sandbox.py index 4dc93922f282..701085534a59 100644 --- a/products/tasks/backend/logic/services/modal_sandbox.py +++ b/products/tasks/backend/logic/services/modal_sandbox.py @@ -55,7 +55,6 @@ from products.tasks.backend.logic.services.agentsh import ( AGENTSH_DAEMON_PORT, BASH_ENV_SCRIPT, - ENV_FILE, ENV_WRAPPER_SCRIPT, SESSION_ID_FILE, _hostname_from_url, @@ -1000,7 +999,7 @@ def _build_agent_server_command( server_cmd = f"bash -c {shlex.quote(wait_for_repo)}" inner = f"cd /scripts && {server_cmd} > /tmp/agent-server.log 2>&1" - initialize_env_file = f"(test -f {ENV_FILE} || env -0 > {ENV_FILE})" + initialize_env_file = f"bash {shlex.quote(BASH_ENV_SCRIPT)}" if allowed_domains is not None: return ( diff --git a/products/tasks/backend/logic/services/tests/test_modal_sandbox.py b/products/tasks/backend/logic/services/tests/test_modal_sandbox.py index 213dada6ded4..679265c42090 100644 --- a/products/tasks/backend/logic/services/tests/test_modal_sandbox.py +++ b/products/tasks/backend/logic/services/tests/test_modal_sandbox.py @@ -487,7 +487,7 @@ def test_start_agent_server_wraps_with_agentsh_when_domains_provided(self, mock_ command = _agent_server_launch_command(mock_sandbox.execute) assert "--createPr true" in command assert "agentsh exec --client-timeout 2h --timeout 2h" in command - assert "(test -f /tmp/agent-env || env -0 > /tmp/agent-env)" in command + assert "bash /tmp/agentsh-bash-env.sh" in command assert "/tmp/agentsh-env-wrapper.sh" in command assert "./node_modules/.bin/agent-server" in command @@ -509,7 +509,7 @@ def test_start_agent_server_wraps_with_agentsh_when_domains_empty(self, mock_san command = _agent_server_launch_command(mock_sandbox.execute) assert "--allowedDomains" not in command assert "agentsh exec --client-timeout 2h --timeout 2h" in command - assert "(test -f /tmp/agent-env || env -0 > /tmp/agent-env)" in command + assert "bash /tmp/agentsh-bash-env.sh" in command @pytest.mark.parametrize( ("create_pr", "expected_flag"), diff --git a/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py b/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py index 3d67383f254d..c7f880040b73 100644 --- a/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py +++ b/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py @@ -10,12 +10,7 @@ from posthog.temporal.common.utils import asyncify from products.tasks.backend.constants import SNAPSHOT_KIND_FILESYSTEM, filter_user_sandbox_env_vars -from products.tasks.backend.exceptions import ( - GitHubAuthenticationError, - OAuthTokenError, - SandboxProvisionError, - TaskNotFoundError, -) +from products.tasks.backend.exceptions import GitHubAuthenticationError, OAuthTokenError, TaskNotFoundError from products.tasks.backend.logic.services.agentsh import INFRASTRUCTURE_DOMAINS, _get_debug_only_domains from products.tasks.backend.logic.services.connection_token import ( SANDBOX_JWT_STATE_KID_KEY, @@ -33,7 +28,6 @@ from products.tasks.backend.temporal.oauth import create_oauth_access_token_for_run, create_wizard_oauth_access_token from products.tasks.backend.temporal.observability import emit_agent_log, log_activity_execution from products.tasks.backend.temporal.process_task.sandbox_credentials import ( - initialize_sandbox_env_file, set_git_remote_token, update_sandbox_env_file, ) @@ -594,14 +588,6 @@ def create_sandbox_for_repository(input: CreateSandboxForRepositoryInput) -> Cre increment_sandbox_created("vm" if use_vm_sandbox else "gvisor") - if not initialize_sandbox_env_file(sandbox, prepared.environment_variables): - sandbox.destroy() - raise SandboxProvisionError( - "Failed to initialize sandbox environment", - {"task_id": ctx.task_id, "run_id": ctx.run_id, "sandbox_id": sandbox.id}, - cause=RuntimeError("sandbox environment file initialization failed"), - ) - credentials = sandbox.get_connect_credentials() try: diff --git a/products/tasks/backend/temporal/process_task/sandbox_credentials.py b/products/tasks/backend/temporal/process_task/sandbox_credentials.py index bee5f243e314..1b37fbcd18ba 100644 --- a/products/tasks/backend/temporal/process_task/sandbox_credentials.py +++ b/products/tasks/backend/temporal/process_task/sandbox_credentials.py @@ -118,23 +118,6 @@ def update_sandbox_env_file(sandbox: "SandboxBase", updates: dict[str, str]) -> return True -def initialize_sandbox_env_file(sandbox: "SandboxBase", environment_variables: dict[str, str]) -> bool: - """Capture the sandbox environment, then overlay the values resolved for this run. - - This runs synchronously during provisioning so the agent cannot start before its - per-command shells have the same credentials as the agent-server process. - """ - capture = sandbox.execute(f"env -0 > {shlex.quote(ENV_FILE)}", timeout_seconds=30) - if capture.exit_code != 0: - logger.warning( - "Failed to initialize sandbox env file", - extra={"sandbox_id": sandbox.id, "env_file": ENV_FILE, "stderr": capture.stderr}, - ) - return False - - return update_sandbox_env_file(sandbox, environment_variables) - - def apply_github_credentials_to_sandbox(sandbox: "SandboxBase", repository: str | None, github_token: str) -> None: """Re-inject a GitHub token into both places a running sandbox reads it from.""" if repository: diff --git a/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py b/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py index d38e5aa18ba9..014fd645ff43 100644 --- a/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py +++ b/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py @@ -10,7 +10,6 @@ GitHubSandboxCredential, build_sandbox_credentials, github_refresh_interval_seconds, - initialize_sandbox_env_file, set_git_remote_token, update_sandbox_env_file, ) @@ -103,29 +102,6 @@ def test_writes_updates_when_env_file_absent(self): assert payload == b"GH_TOKEN=ghs_new\x00" -class TestInitializeSandboxEnvFile: - def test_captures_container_environment_then_overlays_resolved_credentials(self): - sandbox = MagicMock() - existing = b"PATH=/usr/bin\x00GITHUB_TOKEN=stale\x00" - sandbox.execute.side_effect = [_ok(), _ok(base64.b64encode(existing).decode())] - sandbox.write_file.return_value = _ok() - - assert ( - initialize_sandbox_env_file( - sandbox, - {"GITHUB_TOKEN": "ghu_fresh", "GH_TOKEN": "ghu_fresh"}, - ) - is True - ) - - assert sandbox.execute.call_args_list[0].args[0] == "env -0 > /tmp/agent-env" - _, payload = sandbox.write_file.call_args[0] - entries = {entry.split(b"=", 1)[0]: entry.split(b"=", 1)[1] for entry in payload.split(b"\x00") if entry} - assert entries[b"PATH"] == b"/usr/bin" - assert entries[b"GITHUB_TOKEN"] == b"ghu_fresh" - assert entries[b"GH_TOKEN"] == b"ghu_fresh" - - class TestGitHubSandboxCredential: def test_resolves_and_applies_token_and_reports_interval(self): sandbox = MagicMock() diff --git a/products/tasks/backend/tests/test_agentsh.py b/products/tasks/backend/tests/test_agentsh.py index bf3b805e3dc5..bf646d6d5e5f 100644 --- a/products/tasks/backend/tests/test_agentsh.py +++ b/products/tasks/backend/tests/test_agentsh.py @@ -1,9 +1,13 @@ +import os import shlex +import tempfile +import subprocess +from pathlib import Path from typing import Any from unittest.mock import Mock -from django.test import TestCase, override_settings +from django.test import SimpleTestCase, TestCase, override_settings import yaml from parameterized import parameterized @@ -14,6 +18,7 @@ INFRASTRUCTURE_DOMAINS, build_audit_query_command, build_exec_prefix, + generate_bash_env_script, generate_config_yaml, generate_env_wrapper, generate_policy_yaml, @@ -243,6 +248,52 @@ def test_wrapper_does_not_set_proxy_vars(self): self.assertNotIn("--use-env-proxy", wrapper) +class TestBashEnvScript(SimpleTestCase): + def test_initialization_does_not_overwrite_refreshed_credentials(self): + with tempfile.TemporaryDirectory() as temp_dir: + env_file = Path(temp_dir) / "agent env" + script_file = Path(temp_dir) / "bash env.sh" + refreshed = b"GH_TOKEN=ghu_fresh\x00GITHUB_TOKEN=ghu_fresh\x00" + env_file.write_bytes(refreshed) + script_file.write_text(generate_bash_env_script(str(env_file))) + + subprocess.run( + ["bash", str(script_file)], + check=True, + env={"PATH": os.environ["PATH"], "SAFE_BASE": "kept", "GH_TOKEN": "ghu_stale"}, + ) + + self.assertEqual(env_file.read_bytes(), refreshed) + + sourced = subprocess.run( + ["bash", "-c", 'printf "%s|%s" "$GH_TOKEN" "$GITHUB_TOKEN"'], + check=True, + capture_output=True, + text=True, + env={"PATH": os.environ["PATH"], "BASH_ENV": str(script_file)}, + ) + self.assertEqual(sourced.stdout, "ghu_fresh|ghu_fresh") + + def test_initialization_captures_environment_when_file_is_missing(self): + with tempfile.TemporaryDirectory() as temp_dir: + env_file = Path(temp_dir) / "agent env" + script_file = Path(temp_dir) / "bash env.sh" + script_file.write_text(generate_bash_env_script(str(env_file))) + + subprocess.run( + ["bash", str(script_file)], + check=True, + env={"PATH": os.environ["PATH"], "SAFE_BASE": "kept"}, + ) + + entries = { + entry.split(b"=", 1)[0]: entry.split(b"=", 1)[1] + for entry in env_file.read_bytes().split(b"\x00") + if entry + } + self.assertEqual(entries[b"SAFE_BASE"], b"kept") + + class TestBuildAuditQueryCommand(TestCase): def test_references_audit_db(self): cmd = build_audit_query_command() @@ -285,8 +336,7 @@ def test_command_without_domains_skips_agentsh_exec(self): create_pr=True, ) self.assertNotIn("agentsh exec --client-timeout 2h --timeout 2h", cmd) - # Provisioning initializes the env file synchronously; legacy callers only fill a missing file. - self.assertIn("(test -f /tmp/agent-env || env -0 > /tmp/agent-env)", cmd) + self.assertIn("bash /tmp/agentsh-bash-env.sh", cmd) self.assertNotIn(ENV_WRAPPER_SCRIPT, cmd) self.assertIn("nohup", cmd) @@ -387,7 +437,7 @@ def test_command_includes_allowed_domains(self): allowed_domains=["example.com", "api.example.com"], ) self.assertIn("agentsh exec --client-timeout 2h --timeout 2h", cmd) - self.assertIn("(test -f /tmp/agent-env || env -0 > /tmp/agent-env)", cmd) + self.assertIn("bash /tmp/agentsh-bash-env.sh", cmd) self.assertIn(ENV_WRAPPER_SCRIPT, cmd) self.assertIn("--allowedDomains", cmd) self.assertIn("example.com,api.example.com", cmd) From 4ff5ebdc58af04eee89f736fb81938e25f28584d Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 21 Jul 2026 15:41:58 +0100 Subject: [PATCH 3/4] fix(tasks): address sandbox credential review feedback --- .../tasks/backend/logic/services/agentsh.py | 116 +++++++++++++++--- .../backend/logic/services/docker_sandbox.py | 4 +- .../backend/logic/services/local_packages.py | 34 ++++- .../backend/logic/services/modal_sandbox.py | 4 +- .../services/tests/test_local_packages.py | 23 +++- .../activities/provision_sandbox.py | 20 ++- .../test_inject_fresh_tokens_on_resume.py | 60 +++++---- .../process_task/sandbox_credentials.py | 81 ++++++------ .../tests/test_sandbox_credentials.py | 62 ++++++---- products/tasks/backend/tests/test_agentsh.py | 114 +++++++++++++---- 10 files changed, 352 insertions(+), 166 deletions(-) diff --git a/products/tasks/backend/logic/services/agentsh.py b/products/tasks/backend/logic/services/agentsh.py index ea426309259a..93b64cc67704 100644 --- a/products/tasks/backend/logic/services/agentsh.py +++ b/products/tasks/backend/logic/services/agentsh.py @@ -5,12 +5,16 @@ import yaml +from products.tasks.backend.constants import SANDBOX_AGENT_LAUNCH_UNSET_ENV_VARS + AGENTSH_DAEMON_PORT = 18080 SESSION_ID_FILE = "/tmp/agentsh-session-id" ENV_FILE = "/tmp/agent-env" +GITHUB_ENV_FILE = "/tmp/agent-github-env" +OAUTH_ENV_FILE = "/tmp/agent-oauth-env" ENV_WRAPPER_SCRIPT = "/tmp/agentsh-env-wrapper.sh" # Sourced via BASH_ENV on every `bash -c` the agent runs, so git/gh pick up a -# mid-session GitHub credential refresh (the backend rewrites ENV_FILE in place). +# mid-session GitHub credential refresh from its dedicated credential file. BASH_ENV_SCRIPT = "/tmp/agentsh-bash-env.sh" AGENTSH_AUDIT_DB = "/var/lib/agentsh/events.db" INFRASTRUCTURE_DOMAINS = [ @@ -93,7 +97,22 @@ def _get_debug_only_ports() -> list[int]: return ports -def generate_env_wrapper() -> str: +_MANAGED_CREDENTIAL_ENV_KEYS = ("GH_TOKEN", "GITHUB_TOKEN", "POSTHOG_PERSONAL_API_KEY") +_EXCLUDED_AGENT_ENV_KEYS = ( + *SANDBOX_AGENT_LAUNCH_UNSET_ENV_VARS, + "BASH_ENV", + "PROMPT_COMMAND", + "PYTHONSTARTUP", + "PERL5OPT", + "RUBYOPT", +) + + +def generate_env_wrapper( + env_file: str = ENV_FILE, + github_env_file: str = GITHUB_ENV_FILE, + oauth_env_file: str = OAUTH_ENV_FILE, +) -> str: """Generate a wrapper that restores the full sandbox environment. ``agentsh exec`` starts child processes with a heavily stripped @@ -104,40 +123,107 @@ def generate_env_wrapper() -> str: Network policy enforcement happens at the syscall level (ptrace) — it does not depend on proxy environment variables. """ + quoted_env_file = shlex.quote(env_file) + quoted_github_env_file = shlex.quote(github_env_file) + quoted_oauth_env_file = shlex.quote(oauth_env_file) + excluded_names = " ".join((*_MANAGED_CREDENTIAL_ENV_KEYS, *_EXCLUDED_AGENT_ENV_KEYS)) + excluded_entries = "|".join(f"{name}=*" for name in (*_MANAGED_CREDENTIAL_ENV_KEYS, *_EXCLUDED_AGENT_ENV_KEYS)) return f"""\ #!/bin/bash +unset {excluded_names} +while IFS= read -r -d $'\\0' line; do + case "$line" in + {excluded_entries}) ;; + *) export "$line" ;; + esac +done < {quoted_env_file} 2>/dev/null + while IFS= read -r -d $'\\0' line; do - export "$line" -done < {ENV_FILE} + case "$line" in + GH_TOKEN=*|GITHUB_TOKEN=*) export "$line" ;; + esac +done < {quoted_github_env_file} 2>/dev/null + +while IFS= read -r -d $'\\0' line; do + case "$line" in + POSTHOG_PERSONAL_API_KEY=*) export "$line" ;; + esac +done < {quoted_oauth_env_file} 2>/dev/null exec "$@" """ -def generate_bash_env_script(env_file: str = ENV_FILE) -> str: +def generate_bash_env_script( + env_file: str = ENV_FILE, + github_env_file: str = GITHUB_ENV_FILE, + oauth_env_file: str = OAUTH_ENV_FILE, +) -> str: """ Generate the script sourced via ``BASH_ENV`` and used to initialize its env file. - The explicit invocation runs inside the background agent-server launch. It - atomically captures the current sandbox environment only when the env file does - not already exist, so it cannot overwrite credentials refreshed by the backend. - Sourced invocations stay cheap and only export the GitHub credentials needed by - tool shells. + The explicit invocation runs before the background agent-server launch. It + atomically replaces the full environment with the current sandbox process + environment, excluding launch hooks and credentials. Credential files are + initialized only when absent, so a backend refresh that happened before startup + wins. Sourced invocations stay cheap and only export GitHub credentials. """ quoted_env_file = shlex.quote(env_file) + quoted_github_env_file = shlex.quote(github_env_file) + quoted_oauth_env_file = shlex.quote(oauth_env_file) + excluded_entries = "|".join(f"{name}=*" for name in (*_MANAGED_CREDENTIAL_ENV_KEYS, *_EXCLUDED_AGENT_ENV_KEYS)) return f"""\ if [[ "${{BASH_SOURCE[0]}}" == "$0" ]]; then - tmp_file={quoted_env_file}.tmp.$$ - trap 'rm -f "$tmp_file"' EXIT - env -0 > "$tmp_file" - ln "$tmp_file" {quoted_env_file} 2>/dev/null || true + set -euo pipefail + umask 077 + env_tmp="$(mktemp {quoted_env_file}.tmp.XXXXXX)" + github_tmp="$(mktemp {quoted_github_env_file}.tmp.XXXXXX)" + oauth_tmp="$(mktemp {quoted_oauth_env_file}.tmp.XXXXXX)" + trap 'rm -f "$env_tmp" "$github_tmp" "$oauth_tmp"' EXIT + + while IFS= read -r -d $'\\0' kv 2>/dev/null; do + case "$kv" in + {excluded_entries}) ;; + *) printf '%s\\0' "$kv" >> "$env_tmp" ;; + esac + done < <(env -0) + chmod 600 "$env_tmp" + mv "$env_tmp" {quoted_env_file} + + github_token="${{GITHUB_TOKEN:-${{GH_TOKEN:-}}}}" + if [[ -n "$github_token" ]]; then + printf 'GITHUB_TOKEN=%s\\0GH_TOKEN=%s\\0' "$github_token" "$github_token" > "$github_tmp" + fi + chmod 600 "$github_tmp" + if [[ -e {quoted_github_env_file} || -L {quoted_github_env_file} ]]; then + [[ -f {quoted_github_env_file} && ! -L {quoted_github_env_file} ]] + chmod 600 {quoted_github_env_file} + else + if ! ln "$github_tmp" {quoted_github_env_file} 2>/dev/null; then + [[ -f {quoted_github_env_file} && ! -L {quoted_github_env_file} ]] + fi + fi + + if [[ -n "${{POSTHOG_PERSONAL_API_KEY:-}}" ]]; then + printf 'POSTHOG_PERSONAL_API_KEY=%s\\0' "$POSTHOG_PERSONAL_API_KEY" > "$oauth_tmp" + fi + chmod 600 "$oauth_tmp" + if [[ -e {quoted_oauth_env_file} || -L {quoted_oauth_env_file} ]]; then + [[ -f {quoted_oauth_env_file} && ! -L {quoted_oauth_env_file} ]] + chmod 600 {quoted_oauth_env_file} + else + if ! ln "$oauth_tmp" {quoted_oauth_env_file} 2>/dev/null; then + [[ -f {quoted_oauth_env_file} && ! -L {quoted_oauth_env_file} ]] + fi + fi exit 0 fi +unset GH_TOKEN GITHUB_TOKEN while IFS= read -r -d $'\\0' kv 2>/dev/null; do case "$kv" in GH_TOKEN=*|GITHUB_TOKEN=*) export "$kv" ;; esac -done < {quoted_env_file} 2>/dev/null +done < {quoted_github_env_file} 2>/dev/null """ diff --git a/products/tasks/backend/logic/services/docker_sandbox.py b/products/tasks/backend/logic/services/docker_sandbox.py index 299b69cfe506..598ae9db63bd 100644 --- a/products/tasks/backend/logic/services/docker_sandbox.py +++ b/products/tasks/backend/logic/services/docker_sandbox.py @@ -856,10 +856,10 @@ def _build_agent_server_command( if allowed_domains is not None: return ( f"cd /scripts && {initialize_env_file} && " - f"{build_exec_prefix()} {ENV_WRAPPER_SCRIPT} bash -c {shlex.quote(inner)} &" + f"({build_exec_prefix()} {ENV_WRAPPER_SCRIPT} bash -c {shlex.quote(inner)} &)" ) else: - return f"cd /scripts && {initialize_env_file} && nohup {server_cmd} > /tmp/agent-server.log 2>&1 &" + return f"cd /scripts && {initialize_env_file} && (nohup {server_cmd} > /tmp/agent-server.log 2>&1 &)" def _launch_and_check(self, command: str) -> bool: """Execute the agent-server command and wait for the health check. diff --git a/products/tasks/backend/logic/services/local_packages.py b/products/tasks/backend/logic/services/local_packages.py index d3bc083509c9..e86219b7346c 100644 --- a/products/tasks/backend/logic/services/local_packages.py +++ b/products/tasks/backend/logic/services/local_packages.py @@ -83,7 +83,7 @@ def get_local_posthog_code_packages() -> tuple[LocalPackage, ...] | None: def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) -> dict[str, dict[str, str]]: """Collect registry dependencies needed by the overlaid local package builds.""" dependencies: dict[str, dict[str, str]] = {} - dependency_catalogs: dict[Path, dict[str, str]] = {} + dependency_catalogs: dict[Path, tuple[dict[str, str], dict[str, dict[str, str]]]] = {} for package in packages: manifest_path = package.source_path / "package.json" @@ -98,7 +98,7 @@ def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) - raise ValueError(f"Expected dependency names and versions to be strings in {manifest_path}") if version.startswith("workspace:"): continue - if version == "catalog:": + if version.startswith("catalog:"): workspace_manifest_path = next( ( parent / "pnpm-workspace.yaml" @@ -112,7 +112,8 @@ def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) - if workspace_manifest_path not in dependency_catalogs: workspace_manifest = yaml.safe_load(workspace_manifest_path.read_text()) - catalog = workspace_manifest.get("catalog") if isinstance(workspace_manifest, dict) else None + catalog = workspace_manifest.get("catalog", {}) if isinstance(workspace_manifest, dict) else None + catalogs = workspace_manifest.get("catalogs", {}) if isinstance(workspace_manifest, dict) else None if not isinstance(catalog, dict) or not all( isinstance(catalog_name, str) and isinstance(catalog_version, str) for catalog_name, catalog_version in catalog.items() @@ -120,9 +121,30 @@ def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) - raise ValueError( f"Expected catalog to contain string dependencies in {workspace_manifest_path}" ) - dependency_catalogs[workspace_manifest_path] = catalog - - resolved_version = dependency_catalogs[workspace_manifest_path].get(name) + if not isinstance(catalogs, dict) or not all( + isinstance(catalog_name, str) + and isinstance(named_catalog, dict) + and all( + isinstance(dependency_name, str) and isinstance(dependency_version, str) + for dependency_name, dependency_version in named_catalog.items() + ) + for catalog_name, named_catalog in catalogs.items() + ): + raise ValueError( + f"Expected catalogs to contain named string dependencies in {workspace_manifest_path}" + ) + dependency_catalogs[workspace_manifest_path] = (catalog, catalogs) + + catalog, catalogs = dependency_catalogs[workspace_manifest_path] + catalog_reference = version.removeprefix("catalog:") + if catalog_reference in {"", "*"}: + selected_catalog = catalog + else: + selected_catalog = catalogs.get(catalog_reference) + if selected_catalog is None: + raise ValueError(f"Catalog {catalog_reference} is missing from {workspace_manifest_path}") + + resolved_version = selected_catalog.get(name) if resolved_version is None: raise ValueError(f"Catalog dependency {name} is missing from {workspace_manifest_path}") version = resolved_version diff --git a/products/tasks/backend/logic/services/modal_sandbox.py b/products/tasks/backend/logic/services/modal_sandbox.py index 701085534a59..61f3b55f87c7 100644 --- a/products/tasks/backend/logic/services/modal_sandbox.py +++ b/products/tasks/backend/logic/services/modal_sandbox.py @@ -1004,10 +1004,10 @@ def _build_agent_server_command( if allowed_domains is not None: return ( f"cd /scripts && {initialize_env_file} && " - f"{build_exec_prefix()} {ENV_WRAPPER_SCRIPT} bash -c {shlex.quote(inner)} &" + f"({build_exec_prefix()} {ENV_WRAPPER_SCRIPT} bash -c {shlex.quote(inner)} &)" ) else: - return f"cd /scripts && {initialize_env_file} && nohup {server_cmd} > /tmp/agent-server.log 2>&1 &" + return f"cd /scripts && {initialize_env_file} && (nohup {server_cmd} > /tmp/agent-server.log 2>&1 &)" def _diagnose_startup_failure(self, allowed_domains: list[str] | None) -> dict[str, str]: diagnostics: dict[str, str] = {} diff --git a/products/tasks/backend/logic/services/tests/test_local_packages.py b/products/tasks/backend/logic/services/tests/test_local_packages.py index a7d0eb9f8a45..944634707c68 100644 --- a/products/tasks/backend/logic/services/tests/test_local_packages.py +++ b/products/tasks/backend/logic/services/tests/test_local_packages.py @@ -72,10 +72,22 @@ def test_returns_packages_when_everything_present(self, fake_monorepo: Path) -> def test_resolves_default_catalog_runtime_dependencies(fake_monorepo: Path) -> None: - (fake_monorepo / "pnpm-workspace.yaml").write_text("catalog:\n catalog-runtime: 1.2.3\n") + (fake_monorepo / "pnpm-workspace.yaml").write_text( + "catalog:\n" + " catalog-runtime: 1.2.3\n" + " star-catalog-runtime: 2.3.4\n" + "catalogs:\n" + " build:\n" + " named-catalog-runtime: 3.4.5\n" + ) agent_source_path = fake_monorepo / "packages" / "agent" (agent_source_path / "package.json").write_text( - '{"dependencies":{"catalog-runtime":"catalog:","registry-runtime":"^4.5.6"}}' + '{"dependencies":{' + '"catalog-runtime":"catalog:",' + '"star-catalog-runtime":"catalog:*",' + '"named-catalog-runtime":"catalog:build",' + '"registry-runtime":"^4.5.6"' + "}}" ) package = LocalPackage( name="agent", @@ -84,5 +96,10 @@ def test_resolves_default_catalog_runtime_dependencies(fake_monorepo: Path) -> N ) assert get_local_package_runtime_dependencies((package,)) == { - "agent": {"catalog-runtime": "1.2.3", "registry-runtime": "^4.5.6"} + "agent": { + "catalog-runtime": "1.2.3", + "named-catalog-runtime": "3.4.5", + "registry-runtime": "^4.5.6", + "star-catalog-runtime": "2.3.4", + } } diff --git a/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py b/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py index c7f880040b73..c3303a71c893 100644 --- a/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py +++ b/products/tasks/backend/temporal/process_task/activities/provision_sandbox.py @@ -28,8 +28,8 @@ from products.tasks.backend.temporal.oauth import create_oauth_access_token_for_run, create_wizard_oauth_access_token from products.tasks.backend.temporal.observability import emit_agent_log, log_activity_execution from products.tasks.backend.temporal.process_task.sandbox_credentials import ( + replace_sandbox_credentials, set_git_remote_token, - update_sandbox_env_file, ) from products.tasks.backend.temporal.process_task.utils import ( get_git_identity_env_vars, @@ -759,19 +759,13 @@ def inject_fresh_tokens_on_resume(input: InjectFreshTokensOnResumeInput) -> None sandbox = Sandbox.get_by_id(input.sandbox_id) - if github_token and input.repository: - set_git_remote_token(sandbox, input.repository, github_token) + if input.repository: + set_git_remote_token(sandbox, input.repository, github_token or None) - # Update the initialized env file before any resumed command can observe - # credentials persisted in the snapshot. - fresh_env_vars: dict[str, str] = {} - if github_token: - fresh_env_vars["GITHUB_TOKEN"] = github_token - fresh_env_vars["GH_TOKEN"] = github_token - if access_token: - fresh_env_vars["POSTHOG_PERSONAL_API_KEY"] = access_token - - update_sandbox_env_file(sandbox, fresh_env_vars) + # Replace both credential domains even when resolution returns no token, + # so revoked credentials cannot survive in a resumed filesystem snapshot. + if not replace_sandbox_credentials(sandbox, github_token or None, access_token or None): + raise RuntimeError("Failed to replace resumed sandbox credentials") emit_agent_log(ctx.run_id, "debug", "Refreshed sandbox credentials after resume") diff --git a/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py b/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py index 83677e338a8b..0740982fea72 100644 --- a/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py +++ b/products/tasks/backend/temporal/process_task/activities/tests/test_inject_fresh_tokens_on_resume.py @@ -1,5 +1,3 @@ -import base64 - import pytest from unittest.mock import MagicMock, patch @@ -7,7 +5,7 @@ from posthog.models import OrganizationMembership, User -from products.tasks.backend.logic.services.agentsh import ENV_FILE +from products.tasks.backend.logic.services.agentsh import GITHUB_ENV_FILE, OAUTH_ENV_FILE from products.tasks.backend.logic.services.sandbox import ExecutionResult from products.tasks.backend.models import SandboxEnvironment from products.tasks.backend.temporal.process_task.activities.get_task_processing_context import TaskProcessingContext @@ -29,12 +27,9 @@ def sandbox(self): fake.write_file.return_value = ExecutionResult(stdout="", stderr="", exit_code=0) return fake - def test_refreshes_git_remote_url_and_env_file(self, activity_environment, task_context, test_task, sandbox): - existing = b"PATH=/usr/bin\x00GITHUB_TOKEN=ghs_old\x00" - sandbox.execute.side_effect = [ - ExecutionResult(stdout="", stderr="", exit_code=0), - ExecutionResult(stdout=base64.b64encode(existing).decode(), stderr="", exit_code=0), - ] + def test_refreshes_git_remote_url_and_credential_files( + self, activity_environment, task_context, test_task, sandbox + ): with ( patch( "products.tasks.backend.temporal.process_task.activities.provision_sandbox.Sandbox.get_by_id", @@ -58,24 +53,22 @@ def test_refreshes_git_remote_url_and_env_file(self, activity_environment, task_ ), ) - assert sandbox.execute.call_count == 2 + assert sandbox.execute.call_count == 3 remote_command = sandbox.execute.call_args_list[0].args[0] assert "git remote set-url origin" in remote_command assert "x-access-token:ghs_new" in remote_command assert task_context.repository in remote_command - assert sandbox.write_file.call_count == 1 - path, payload = sandbox.write_file.call_args[0] - assert path == ENV_FILE - decoded = payload.decode() - # Null-separated `env -0` format. - assert "\x00" in decoded - assert "GITHUB_TOKEN=ghs_new" in decoded - assert "GH_TOKEN=ghs_new" in decoded - assert "POSTHOG_PERSONAL_API_KEY=oauth_new" in decoded - assert "PATH=/usr/bin" in decoded + assert [call.args for call in sandbox.write_file.call_args_list] == [ + (GITHUB_ENV_FILE, b"GITHUB_TOKEN=ghs_new\x00GH_TOKEN=ghs_new\x00"), + (OAUTH_ENV_FILE, b"POSTHOG_PERSONAL_API_KEY=oauth_new\x00"), + ] + assert [call.args[0] for call in sandbox.execute.call_args_list[1:]] == [ + f"chmod 600 {GITHUB_ENV_FILE}", + f"chmod 600 {OAUTH_ENV_FILE}", + ] - def test_skips_git_remote_when_github_integration_missing(self, activity_environment, test_task, sandbox): + def test_clears_stale_github_credentials_when_integration_missing(self, activity_environment, test_task, sandbox): context = TaskProcessingContext( task_id=str(test_task.id), run_id="run-id", @@ -106,16 +99,21 @@ def test_skips_git_remote_when_github_integration_missing(self, activity_environ ), ) - assert sandbox.execute.call_count == 1 - # OAuth env is still written so POSTHOG_PERSONAL_API_KEY refreshes. - assert sandbox.write_file.call_count == 1 - _, payload = sandbox.write_file.call_args[0] - decoded = payload.decode() - assert "POSTHOG_PERSONAL_API_KEY=oauth_new" in decoded - assert "GITHUB_TOKEN" not in decoded + assert sandbox.execute.call_count == 3 + remote_command = sandbox.execute.call_args_list[0].args[0] + assert "https://github.com/" in remote_command + assert "x-access-token" not in remote_command + assert [call.args for call in sandbox.write_file.call_args_list] == [ + (GITHUB_ENV_FILE, b""), + (OAUTH_ENV_FILE, b"POSTHOG_PERSONAL_API_KEY=oauth_new\x00"), + ] def test_logs_warning_when_remote_url_update_fails(self, activity_environment, task_context, test_task, sandbox): - sandbox.execute.return_value = ExecutionResult(stdout="", stderr="fatal: not a git repository", exit_code=128) + sandbox.execute.side_effect = [ + ExecutionResult(stdout="", stderr="fatal: not a git repository", exit_code=128), + ExecutionResult(stdout="", stderr="", exit_code=0), + ExecutionResult(stdout="", stderr="", exit_code=0), + ] with ( patch( @@ -141,8 +139,8 @@ def test_logs_warning_when_remote_url_update_fails(self, activity_environment, t ), ) - assert sandbox.execute.call_count == 2 - assert sandbox.write_file.call_count == 1 + assert sandbox.execute.call_count == 3 + assert sandbox.write_file.call_count == 2 def test_prepare_sandbox_injects_user_github_token_without_repository(self, activity_environment, team, user): from products.tasks.backend.models import Task diff --git a/products/tasks/backend/temporal/process_task/sandbox_credentials.py b/products/tasks/backend/temporal/process_task/sandbox_credentials.py index 1b37fbcd18ba..976e494392bc 100644 --- a/products/tasks/backend/temporal/process_task/sandbox_credentials.py +++ b/products/tasks/backend/temporal/process_task/sandbox_credentials.py @@ -1,7 +1,6 @@ """Refresh long-lived credentials inside a running task sandbox by re-resolving and re-applying tokens in place.""" import shlex -import base64 import logging from dataclasses import dataclass from typing import TYPE_CHECKING, Protocol @@ -13,7 +12,7 @@ from posthog.redis import get_client from products.tasks.backend.exceptions import CredentialUnavailableError -from products.tasks.backend.logic.services.agentsh import ENV_FILE +from products.tasks.backend.logic.services.agentsh import GITHUB_ENV_FILE, OAUTH_ENV_FILE from products.tasks.backend.models import Task, TaskRun from products.tasks.backend.temporal.process_task.utils import ( PrAuthorshipMode, @@ -35,6 +34,7 @@ logger = logging.getLogger(__name__) GITHUB_ENV_KEYS = ("GITHUB_TOKEN", "GH_TOKEN") +OAUTH_ENV_KEY = "POSTHOG_PERSONAL_API_KEY" # Refresh at half the token's server-side half-life so the in-sandbox copy never lapses mid-run. # ghs_ = installation token (~1h) → 20 min; ghu_ = user-to-server token (~8h) → 2 h @@ -52,15 +52,18 @@ def github_refresh_interval_seconds(token: str) -> float: return DEFAULT_REFRESH_INTERVAL_SECONDS -def set_git_remote_token(sandbox: "SandboxBase", repository: str, github_token: str) -> bool: - """Rewrite ``origin``'s remote URL with a fresh ``x-access-token``; git re-reads it on every op. No-ops pre-clone.""" +def set_git_remote_token(sandbox: "SandboxBase", repository: str, github_token: str | None) -> bool: + """Rewrite ``origin`` with the current credential state; git re-reads it on every operation.""" org, repo = repository.lower().split("/") repo_path = f"/tmp/workspace/repos/{org}/{repo}" + if github_token: + remote_url = f"https://x-access-token:{github_token}@github.com/{repository}.git" + else: + remote_url = f"https://github.com/{repository}.git" update_remote = ( f"if [ -d {shlex.quote(repo_path + '/.git')} ]; then " f"cd {shlex.quote(repo_path)} && " - f"git remote set-url origin " - f"https://x-access-token:{shlex.quote(github_token)}@github.com/{shlex.quote(repository)}.git; " + f"git remote set-url origin {shlex.quote(remote_url)}; " f"fi" ) result = sandbox.execute(update_remote, timeout_seconds=30) @@ -73,56 +76,44 @@ def set_git_remote_token(sandbox: "SandboxBase", repository: str, github_token: return True -def update_sandbox_env_file(sandbox: "SandboxBase", updates: dict[str, str]) -> bool: - """Replace specific keys in the NUL-delimited agentsh env file, preserving all other entries. - - Read-modify-write via base64 to survive NUL bytes. The exec wrapper re-sources the - file per command, so updates reach the agent's later ``gh``/``git`` calls without a reboot. - """ - if not updates: - return True - - read = sandbox.execute(f"base64 -w0 {shlex.quote(ENV_FILE)} 2>/dev/null || true", timeout_seconds=30) - existing: bytes = b"" - if read.exit_code == 0 and read.stdout.strip(): - try: - existing = base64.b64decode(read.stdout.strip()) - except Exception: - logger.warning("Could not decode existing sandbox env file; rewriting only the updated keys") - existing = b"" - - ordered_keys: list[str] = [] - values: dict[str, bytes] = {} - for entry in existing.split(b"\x00"): - if not entry: - continue - key_bytes, _, value_bytes = entry.partition(b"=") - key = key_bytes.decode("utf-8", "replace") - if key not in values: - ordered_keys.append(key) - values[key] = value_bytes - - for key, value in updates.items(): - if key not in values: - ordered_keys.append(key) - values[key] = value.encode("utf-8") - - payload = b"".join(f"{key}=".encode() + values[key] + b"\x00" for key in ordered_keys) - write = sandbox.write_file(ENV_FILE, payload) +def _write_sandbox_credential_file(sandbox: "SandboxBase", path: str, payload: bytes) -> bool: + """Atomically replace one credential domain without a cross-key read-modify-write.""" + write = sandbox.write_file(path, payload) if write.exit_code != 0: logger.warning( - "Failed to refresh agentsh env file", - extra={"sandbox_id": sandbox.id, "env_file": ENV_FILE, "stderr": write.stderr}, + "Failed to refresh sandbox credential file", + extra={"sandbox_id": sandbox.id, "credential_file": path, "stderr": write.stderr}, + ) + return False + + chmod = sandbox.execute(f"chmod 600 {shlex.quote(path)}", timeout_seconds=30) + if chmod.exit_code != 0: + logger.warning( + "Failed to restrict sandbox credential file permissions", + extra={"sandbox_id": sandbox.id, "credential_file": path, "stderr": chmod.stderr}, ) return False return True +def replace_sandbox_credentials( + sandbox: "SandboxBase", github_token: str | None, oauth_access_token: str | None +) -> bool: + """Replace every managed credential, including empty values that revoke stale snapshot state.""" + github_payload = b"".join(f"{key}={github_token}\x00".encode() for key in GITHUB_ENV_KEYS) if github_token else b"" + oauth_payload = f"{OAUTH_ENV_KEY}={oauth_access_token}\x00".encode() if oauth_access_token else b"" + + github_updated = _write_sandbox_credential_file(sandbox, GITHUB_ENV_FILE, github_payload) + oauth_updated = _write_sandbox_credential_file(sandbox, OAUTH_ENV_FILE, oauth_payload) + return github_updated and oauth_updated + + def apply_github_credentials_to_sandbox(sandbox: "SandboxBase", repository: str | None, github_token: str) -> None: """Re-inject a GitHub token into both places a running sandbox reads it from.""" if repository: set_git_remote_token(sandbox, repository, github_token) - update_sandbox_env_file(sandbox, dict.fromkeys(GITHUB_ENV_KEYS, github_token)) + github_payload = b"".join(f"{key}={github_token}\x00".encode() for key in GITHUB_ENV_KEYS) + _write_sandbox_credential_file(sandbox, GITHUB_ENV_FILE, github_payload) USER_TOKEN_REFRESH_INTERVAL_SECONDS: float = _GITHUB_REFRESH_INTERVAL_BY_PREFIX["ghu_"] diff --git a/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py b/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py index 014fd645ff43..87a68d11dcb0 100644 --- a/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py +++ b/products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py @@ -1,8 +1,7 @@ -import base64 - import pytest from unittest.mock import MagicMock, patch +from products.tasks.backend.logic.services.agentsh import GITHUB_ENV_FILE, OAUTH_ENV_FILE from products.tasks.backend.logic.services.sandbox import ExecutionResult from products.tasks.backend.temporal.process_task.activities.get_task_processing_context import TaskProcessingContext from products.tasks.backend.temporal.process_task.sandbox_credentials import ( @@ -10,8 +9,8 @@ GitHubSandboxCredential, build_sandbox_credentials, github_refresh_interval_seconds, + replace_sandbox_credentials, set_git_remote_token, - update_sandbox_env_file, ) @@ -60,6 +59,16 @@ def test_rewrites_remote_with_fresh_token(self): assert "x-access-token:ghs_new" in command assert "explore-science/paper-wizard-frontend" in command + def test_removes_stale_token_when_current_credential_is_missing(self): + sandbox = MagicMock() + sandbox.execute.return_value = _ok() + + assert set_git_remote_token(sandbox, "owner/repo", None) is True + + command = sandbox.execute.call_args[0][0] + assert "https://github.com/owner/repo.git" in command + assert "x-access-token" not in command + def test_returns_false_on_failure(self): sandbox = MagicMock() sandbox.execute.return_value = ExecutionResult(stdout="", stderr="not a git repo", exit_code=128) @@ -67,39 +76,38 @@ def test_returns_false_on_failure(self): assert set_git_remote_token(sandbox, "owner/repo", "ghs_new") is False -class TestUpdateSandboxEnvFile: - def test_preserves_other_keys_and_replaces_updated_ones(self): +class TestReplaceSandboxCredentials: + def test_replaces_each_credential_domain_without_reading_existing_state(self): sandbox = MagicMock() - existing = b"PATH=/usr/bin\x00GITHUB_TOKEN=ghs_old\x00HOME=/root\x00" - sandbox.execute.return_value = _ok(base64.b64encode(existing).decode()) + sandbox.execute.return_value = _ok() sandbox.write_file.return_value = _ok() - assert update_sandbox_env_file(sandbox, {"GITHUB_TOKEN": "ghs_new", "GH_TOKEN": "ghs_new"}) is True + assert replace_sandbox_credentials(sandbox, "ghs_new", "oauth_new") is True - _, payload = sandbox.write_file.call_args[0] - entries = {e.split(b"=", 1)[0]: e.split(b"=", 1)[1] for e in payload.split(b"\x00") if e} - # Untouched keys survive, updated key is replaced, new key is appended. - assert entries[b"PATH"] == b"/usr/bin" - assert entries[b"HOME"] == b"/root" - assert entries[b"GITHUB_TOKEN"] == b"ghs_new" - assert entries[b"GH_TOKEN"] == b"ghs_new" - - def test_noop_when_no_updates(self): - sandbox = MagicMock() - assert update_sandbox_env_file(sandbox, {}) is True - sandbox.execute.assert_not_called() - sandbox.write_file.assert_not_called() + assert sandbox.write_file.call_args_list[0].args == ( + GITHUB_ENV_FILE, + b"GITHUB_TOKEN=ghs_new\x00GH_TOKEN=ghs_new\x00", + ) + assert sandbox.write_file.call_args_list[1].args == ( + OAUTH_ENV_FILE, + b"POSTHOG_PERSONAL_API_KEY=oauth_new\x00", + ) + assert [call.args[0] for call in sandbox.execute.call_args_list] == [ + f"chmod 600 {GITHUB_ENV_FILE}", + f"chmod 600 {OAUTH_ENV_FILE}", + ] - def test_writes_updates_when_env_file_absent(self): + def test_empty_current_credentials_clear_both_files(self): sandbox = MagicMock() - # base64 of empty file (the `|| true` path yields empty stdout). - sandbox.execute.return_value = _ok("") + sandbox.execute.return_value = _ok() sandbox.write_file.return_value = _ok() - assert update_sandbox_env_file(sandbox, {"GH_TOKEN": "ghs_new"}) is True + assert replace_sandbox_credentials(sandbox, None, None) is True - _, payload = sandbox.write_file.call_args[0] - assert payload == b"GH_TOKEN=ghs_new\x00" + assert [call.args for call in sandbox.write_file.call_args_list] == [ + (GITHUB_ENV_FILE, b""), + (OAUTH_ENV_FILE, b""), + ] class TestGitHubSandboxCredential: diff --git a/products/tasks/backend/tests/test_agentsh.py b/products/tasks/backend/tests/test_agentsh.py index bf646d6d5e5f..d7e4d8524f14 100644 --- a/products/tasks/backend/tests/test_agentsh.py +++ b/products/tasks/backend/tests/test_agentsh.py @@ -232,14 +232,36 @@ def test_allow_all_policy_has_no_metadata_deny_rule(self): self.assertNotIn("deny-cloud-metadata", rule_names) -class TestEnvWrapper(TestCase): - def test_wrapper_restores_environment_dump(self): - wrapper = generate_env_wrapper() - self.assertIn("done < /tmp/agent-env", wrapper) +class TestEnvWrapper(SimpleTestCase): + def test_restores_safe_environment_and_only_managed_credentials(self): + with tempfile.TemporaryDirectory() as temp_dir: + env_file = Path(temp_dir) / "agent env" + github_env_file = Path(temp_dir) / "github env" + oauth_env_file = Path(temp_dir) / "oauth env" + wrapper_file = Path(temp_dir) / "wrapper.sh" + env_file.write_bytes( + b"SAFE_BASE=kept\x00NODE_OPTIONS=--require=/tmp/payload.js\x00GITHUB_TOKEN=ghs_snapshot\x00" + ) + github_env_file.write_bytes(b"GITHUB_TOKEN=ghs_fresh\x00GH_TOKEN=ghs_fresh\x00IGNORED=unsafe\x00") + oauth_env_file.write_bytes(b"POSTHOG_PERSONAL_API_KEY=oauth_fresh\x00IGNORED=unsafe\x00") + wrapper_file.write_text(generate_env_wrapper(str(env_file), str(github_env_file), str(oauth_env_file))) + + result = subprocess.run( + [ + "bash", + str(wrapper_file), + "bash", + "-c", + 'printf "%s|%s|%s|%s|%s" "$SAFE_BASE" "$GH_TOKEN" "$GITHUB_TOKEN" ' + '"$POSTHOG_PERSONAL_API_KEY" "${NODE_OPTIONS:-}"', + ], + check=True, + capture_output=True, + text=True, + env={"PATH": os.environ["PATH"], "NODE_OPTIONS": "--require=/tmp/inherited.js"}, + ) - def test_wrapper_execs_command(self): - wrapper = generate_env_wrapper() - self.assertIn('exec "$@"', wrapper) + self.assertEqual(result.stdout, "kept|ghs_fresh|ghs_fresh|oauth_fresh|") def test_wrapper_does_not_set_proxy_vars(self): wrapper = generate_env_wrapper() @@ -249,21 +271,44 @@ def test_wrapper_does_not_set_proxy_vars(self): class TestBashEnvScript(SimpleTestCase): - def test_initialization_does_not_overwrite_refreshed_credentials(self): + def test_initialization_replaces_snapshot_env_and_preserves_refreshed_credentials(self): with tempfile.TemporaryDirectory() as temp_dir: env_file = Path(temp_dir) / "agent env" + github_env_file = Path(temp_dir) / "github env" + oauth_env_file = Path(temp_dir) / "oauth env" script_file = Path(temp_dir) / "bash env.sh" - refreshed = b"GH_TOKEN=ghu_fresh\x00GITHUB_TOKEN=ghu_fresh\x00" - env_file.write_bytes(refreshed) - script_file.write_text(generate_bash_env_script(str(env_file))) + env_file.write_bytes( + b"PATH=/snapshot\x00NODE_OPTIONS=--require=/tmp/payload.js\x00GITHUB_TOKEN=ghu_snapshot\x00" + ) + github_env_file.write_bytes(b"GH_TOKEN=ghu_fresh\x00GITHUB_TOKEN=ghu_fresh\x00") + oauth_env_file.write_bytes(b"POSTHOG_PERSONAL_API_KEY=oauth_fresh\x00") + script_file.write_text(generate_bash_env_script(str(env_file), str(github_env_file), str(oauth_env_file))) subprocess.run( ["bash", str(script_file)], check=True, - env={"PATH": os.environ["PATH"], "SAFE_BASE": "kept", "GH_TOKEN": "ghu_stale"}, + env={ + "PATH": os.environ["PATH"], + "SAFE_BASE": "kept", + "GH_TOKEN": "ghu_process", + "POSTHOG_PERSONAL_API_KEY": "oauth_process", + "NODE_OPTIONS": "--require=/tmp/current.js", + }, ) - self.assertEqual(env_file.read_bytes(), refreshed) + entries = { + entry.split(b"=", 1)[0]: entry.split(b"=", 1)[1] + for entry in env_file.read_bytes().split(b"\x00") + if entry + } + self.assertEqual(entries[b"SAFE_BASE"], b"kept") + self.assertNotIn(b"NODE_OPTIONS", entries) + self.assertNotIn(b"GITHUB_TOKEN", entries) + self.assertNotIn(b"POSTHOG_PERSONAL_API_KEY", entries) + self.assertEqual(github_env_file.read_bytes(), b"GH_TOKEN=ghu_fresh\x00GITHUB_TOKEN=ghu_fresh\x00") + self.assertEqual(oauth_env_file.read_bytes(), b"POSTHOG_PERSONAL_API_KEY=oauth_fresh\x00") + for path in (env_file, github_env_file, oauth_env_file): + self.assertEqual(path.stat().st_mode & 0o777, 0o600) sourced = subprocess.run( ["bash", "-c", 'printf "%s|%s" "$GH_TOKEN" "$GITHUB_TOKEN"'], @@ -274,24 +319,49 @@ def test_initialization_does_not_overwrite_refreshed_credentials(self): ) self.assertEqual(sourced.stdout, "ghu_fresh|ghu_fresh") - def test_initialization_captures_environment_when_file_is_missing(self): + def test_initialization_creates_restrictive_credential_files_when_missing(self): with tempfile.TemporaryDirectory() as temp_dir: env_file = Path(temp_dir) / "agent env" + github_env_file = Path(temp_dir) / "github env" + oauth_env_file = Path(temp_dir) / "oauth env" script_file = Path(temp_dir) / "bash env.sh" - script_file.write_text(generate_bash_env_script(str(env_file))) + script_file.write_text(generate_bash_env_script(str(env_file), str(github_env_file), str(oauth_env_file))) subprocess.run( ["bash", str(script_file)], check=True, - env={"PATH": os.environ["PATH"], "SAFE_BASE": "kept"}, + env={ + "PATH": os.environ["PATH"], + "SAFE_BASE": "kept", + "GITHUB_TOKEN": "ghs_current", + "POSTHOG_PERSONAL_API_KEY": "oauth_current", + }, ) - entries = { - entry.split(b"=", 1)[0]: entry.split(b"=", 1)[1] - for entry in env_file.read_bytes().split(b"\x00") - if entry - } - self.assertEqual(entries[b"SAFE_BASE"], b"kept") + self.assertEqual( + github_env_file.read_bytes(), + b"GITHUB_TOKEN=ghs_current\x00GH_TOKEN=ghs_current\x00", + ) + self.assertEqual(oauth_env_file.read_bytes(), b"POSTHOG_PERSONAL_API_KEY=oauth_current\x00") + for path in (env_file, github_env_file, oauth_env_file): + self.assertEqual(path.stat().st_mode & 0o777, 0o600) + + def test_initialization_fails_for_untrusted_credential_file_type(self): + with tempfile.TemporaryDirectory() as temp_dir: + env_file = Path(temp_dir) / "agent env" + github_env_file = Path(temp_dir) / "github env" + oauth_env_file = Path(temp_dir) / "oauth env" + script_file = Path(temp_dir) / "bash env.sh" + github_env_file.mkdir() + script_file.write_text(generate_bash_env_script(str(env_file), str(github_env_file), str(oauth_env_file))) + + result = subprocess.run( + ["bash", str(script_file)], + check=False, + env={"PATH": os.environ["PATH"]}, + ) + + self.assertNotEqual(result.returncode, 0) class TestBuildAuditQueryCommand(TestCase): From 75ab27041d9755c5f504da81c299d32028cb00b0 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 21 Jul 2026 15:46:10 +0100 Subject: [PATCH 4/4] fix(tasks): narrow named catalog types --- products/tasks/backend/logic/services/local_packages.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/products/tasks/backend/logic/services/local_packages.py b/products/tasks/backend/logic/services/local_packages.py index e86219b7346c..2ee70c13ed8d 100644 --- a/products/tasks/backend/logic/services/local_packages.py +++ b/products/tasks/backend/logic/services/local_packages.py @@ -140,9 +140,10 @@ def get_local_package_runtime_dependencies(packages: tuple[LocalPackage, ...]) - if catalog_reference in {"", "*"}: selected_catalog = catalog else: - selected_catalog = catalogs.get(catalog_reference) - if selected_catalog is None: + named_catalog = catalogs.get(catalog_reference) + if named_catalog is None: raise ValueError(f"Catalog {catalog_reference} is missing from {workspace_manifest_path}") + selected_catalog = named_catalog resolved_version = selected_catalog.get(name) if resolved_version is None: