From dba732a30f13f9c607aa0d9feb2273639ed3a55c Mon Sep 17 00:00:00 2001 From: "liyi.ly" Date: Tue, 7 Jul 2026 09:36:13 +0800 Subject: [PATCH 1/2] fix: address P0/P1 review findings across CLI, signing, apps, build - cli_exec: lazy-import termios/tty so the CLI no longer crashes at import time on Windows; interactive exec/shell fails with a clear message instead. - ve_sign: sign X-Security-Token into the canonical headers when an STS session_token is present (mirrors auth/_sigv4); thread the token through ve_request and the VeCR/VeCodePipeline call sites. Static ak/sk callers keep the original four-header form (byte-identical signature). - identity.auth: forward the real STS session_token and stop sending the hard-coded 'identity_token' placeholder; IdentityToken is now an opt-in arg. - agent_server origin: disable allow_credentials when CORS origins are a wildcard (Starlette would otherwise echo any Origin with credentials). - ve_pipeline: honor config.build_timeout instead of the hard-coded 900s. - cli delete credential/harness: add --force and an interactive-only confirmation prompt; non-interactive callers keep deleting without a prompt (no behavior change for CI/scripts). - Tests: STS signing golden vector, CORS credential gating, delete confirmation (interactive decline + non-interactive proceed). --- agentkit/apps/agent_server_app/origin.py | 16 +++++- agentkit/sdk/identity/auth.py | 26 ++++++--- agentkit/toolkit/builders/ve_pipeline.py | 6 +- agentkit/toolkit/cli/cli_delete.py | 48 ++++++++++++++++ agentkit/toolkit/cli/sandbox/cli_exec.py | 19 ++++++- agentkit/toolkit/volcengine/code_pipeline.py | 13 +++++ agentkit/toolkit/volcengine/cr.py | 11 ++++ .../toolkit/volcengine/services/cr_service.py | 1 + agentkit/utils/ve_sign.py | 34 ++++++++---- tests/apps/test_agent_server_origin.py | 22 ++++++++ tests/toolkit/cli/test_cli_credential.py | 55 +++++++++++++++++++ tests/utils/test_ve_sign_signing.py | 35 ++++++++++++ 12 files changed, 262 insertions(+), 24 deletions(-) diff --git a/agentkit/apps/agent_server_app/origin.py b/agentkit/apps/agent_server_app/origin.py index 439980c..66ba656 100644 --- a/agentkit/apps/agent_server_app/origin.py +++ b/agentkit/apps/agent_server_app/origin.py @@ -13,6 +13,7 @@ # limitations under the License. import inspect +import logging import os import re from collections.abc import Callable @@ -21,6 +22,8 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware +logger = logging.getLogger(__name__) + _REGEX_PREFIX = "regex:" DEFAULT_AGENTKIT_ALLOW_ORIGINS = ["*"] @@ -114,11 +117,22 @@ def add_cors_compat_middleware(app: FastAPI, allow_origins: list[str]) -> None: """Add CORS middleware for ADK versions that cannot parse regex origins.""" literal_origins, allow_origin_regex = split_allow_origins(allow_origins) + # A wildcard origin combined with allow_credentials=True is unsafe: Starlette + # then echoes the caller's Origin back with Access-Control-Allow-Credentials, + # which lets *any* site make credentialed cross-origin calls. Only enable + # credentials when the origin set is an explicit allowlist. + allow_credentials = "*" not in literal_origins + if not allow_credentials: + logger.warning( + "CORS allow_origins includes '*'; disabling allow_credentials. Set " + "AGENTKIT_ALLOW_ORIGINS (or allow_origins=) to an explicit allowlist " + "to permit credentialed cross-origin requests." + ) app.add_middleware( CORSMiddleware, allow_origins=literal_origins, allow_origin_regex=allow_origin_regex, - allow_credentials=True, + allow_credentials=allow_credentials, allow_methods=["*"], allow_headers=["*"], ) diff --git a/agentkit/sdk/identity/auth.py b/agentkit/sdk/identity/auth.py index 5971c08..5cb548e 100644 --- a/agentkit/sdk/identity/auth.py +++ b/agentkit/sdk/identity/auth.py @@ -24,12 +24,21 @@ from agentkit.toolkit.errors import ApiError -def requires_api_key(*, provider_name: str, into: str = "api_key"): +def requires_api_key( + *, + provider_name: str, + into: str = "api_key", + identity_token: str | None = None, +): """Decorator that fetches an API key before calling the decorated function. Args: provider_name: The credential provider name into: Parameter name to inject the API key into + identity_token: Workload identity token forwarded to ``GetResourceApiKey`` + as ``IdentityToken``. Supply this when the provider validates it; + when omitted the field is not sent (previously a hard-coded + ``"identity_token"`` placeholder was sent, which never validated). Returns: Decorator function @@ -41,17 +50,20 @@ def _get_api_key() -> str: endpoint = resolve_endpoint("identity") access_key = creds.access_key secret_key = creds.secret_key - session_token = "" + # Forward the STS session token so signing works under temporary + # credentials (ve_request signs X-Security-Token when present). + session_token = creds.session_token or None + + request_body: dict[str, str] = {"ProviderName": provider_name} + if identity_token: + request_body["IdentityToken"] = identity_token response = ve_request( - request_body={ - "ProviderName": provider_name, - "IdentityToken": "identity_token", - }, + request_body=request_body, action="GetResourceApiKey", - header={"X-Security-Token": session_token} if session_token else {}, ak=access_key, sk=secret_key, + session_token=session_token, version=endpoint.api_version, service=endpoint.service, host=endpoint.host, diff --git a/agentkit/toolkit/builders/ve_pipeline.py b/agentkit/toolkit/builders/ve_pipeline.py index d881d51..fba8020 100644 --- a/agentkit/toolkit/builders/ve_pipeline.py +++ b/agentkit/toolkit/builders/ve_pipeline.py @@ -1611,8 +1611,10 @@ def download_and_show_logs(run_id: str): self.reporter.success(f"Pipeline triggered successfully, run ID: {run_id}") self.reporter.info("Waiting for build completion...") - # Wait for build completion using reporter's long task interface - max_wait_time = 900 # 15 minutes + # Wait for build completion using reporter's long task interface. + # Honor the configured build_timeout (default 3600s); large images + # (heavy deps, multi-stage Go builds) routinely exceed 15 minutes. + max_wait_time = config.build_timeout or 900 check_interval = 3 # Check every 3 seconds expected_time = ( 30 # Controls progress curve speed (smaller = faster initial progress) diff --git a/agentkit/toolkit/cli/cli_delete.py b/agentkit/toolkit/cli/cli_delete.py index ae5b534..86b4ba2 100644 --- a/agentkit/toolkit/cli/cli_delete.py +++ b/agentkit/toolkit/cli/cli_delete.py @@ -19,6 +19,7 @@ to its id via ``ListInboundAuthConfigs``. """ +import sys from typing import Optional import typer @@ -33,6 +34,27 @@ ) +def _stdin_is_interactive() -> bool: + """Whether a human is at the keyboard (as opposed to CI / a pipe).""" + try: + return sys.stdin.isatty() + except (AttributeError, ValueError): + return False + + +def _confirm_deletion(message: str, *, force: bool) -> None: + """Guard a destructive delete with a confirmation prompt. + + Only prompts in an interactive terminal: non-interactive callers (CI, + scripts, pipes) keep the historical behavior of deleting without a prompt, + so adding this guard is not a breaking change for automation. Pass + ``--force`` to skip the prompt interactively too. + """ + if force or not _stdin_is_interactive(): + return + typer.confirm(message, abort=True) + + @delete_app.command("credential") def delete_credential_command( name: str = typer.Argument(..., help="Credential name to delete."), @@ -44,6 +66,9 @@ def delete_credential_command( "Defaults to VOLCENGINE_AGENTKIT_REGION/VOLCENGINE_REGION/global config." ), ), + force: bool = typer.Option( + False, "--force", "-f", help="Skip the confirmation prompt." + ), ): """Delete a credential (inbound auth config) by name.""" from agentkit.toolkit.cli.utils import PaginationHelper @@ -73,6 +98,20 @@ def build_request(next_token_val): console.print(f"[red]Error: credential '{name}' not found.[/red]") raise typer.Exit(1) + if len(matches) > 1: + console.print( + f"[yellow]{len(matches)} credentials named '{name}' will be deleted:" + "[/yellow]" + ) + for config in matches: + console.print(f" - {config.inbound_auth_config_id}") + + _confirm_deletion( + f"Delete credential '{name}' ({len(matches)} config(s))? " + "This cannot be undone.", + force=force, + ) + for config in matches: config_id = config.inbound_auth_config_id if not config_id: @@ -98,6 +137,9 @@ def delete_harness_command( timeout: int = typer.Option( 300, "--timeout", help="Max seconds to wait for the async deletion to finish." ), + force: bool = typer.Option( + False, "--force", "-f", help="Skip the confirmation prompt." + ), ): """Delete a harness runtime by name. @@ -151,6 +193,12 @@ def _is_harness(runtime) -> bool: ) raise typer.Exit(1) + _confirm_deletion( + f"Delete harness '{name}' ({len(harness_matches)} runtime(s))? " + "This cannot be undone.", + force=force, + ) + for runtime in harness_matches: runtime_id = runtime.runtime_id console.print( diff --git a/agentkit/toolkit/cli/sandbox/cli_exec.py b/agentkit/toolkit/cli/sandbox/cli_exec.py index da810d0..2dfbaed 100644 --- a/agentkit/toolkit/cli/sandbox/cli_exec.py +++ b/agentkit/toolkit/cli/sandbox/cli_exec.py @@ -23,10 +23,17 @@ import shutil import signal import sys -import termios import threading -import tty from contextlib import contextmanager + +try: + # POSIX-only terminal APIs; absent on Windows. Import lazily so that merely + # importing the CLI (e.g. `agentkit --help`) does not crash on Windows. + import termios + import tty +except ImportError: # pragma: no cover - exercised only on Windows + termios = None # type: ignore[assignment] + tty = None # type: ignore[assignment] from pathlib import Path from typing import Iterator, Optional @@ -77,6 +84,14 @@ def _terminal_size() -> dict[str, int]: @contextmanager def _raw_terminal_mode() -> Iterator[None]: + if termios is None or tty is None: + typer.echo( + "Interactive sandbox exec/shell requires a POSIX terminal " + "(termios/tty) and is not supported on this platform.", + err=True, + ) + raise typer.Exit(1) + if not sys.stdin.isatty(): yield return diff --git a/agentkit/toolkit/volcengine/code_pipeline.py b/agentkit/toolkit/volcengine/code_pipeline.py index 391c870..b884e27 100644 --- a/agentkit/toolkit/volcengine/code_pipeline.py +++ b/agentkit/toolkit/volcengine/code_pipeline.py @@ -35,6 +35,7 @@ def __init__( secret_key: str = "", region: str = "", provider: str | None = None, + session_token: str | None = None, ) -> None: # Use provided region or None to trigger auto-detection in VolcConfiguration config = VolcConfiguration( @@ -49,6 +50,8 @@ def __init__( creds = config.get_service_credentials("cp") self.volcengine_access_key = creds.access_key self.volcengine_secret_key = creds.secret_key + # STS credentials carry a session token that must be signed in. + self.session_token = creds.session_token elif not all([access_key, secret_key]): raise ValueError( "Error create cp instance: missing access key or secret key", @@ -56,6 +59,7 @@ def __init__( else: self.volcengine_access_key = access_key self.volcengine_secret_key = secret_key + self.session_token = session_token endpoint = config.get_service_endpoint("cp") @@ -73,6 +77,7 @@ def _get_default_workspace(self) -> str: action="GetDefaultWorkspaceInner", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -143,6 +148,7 @@ def create_workspace( action="CreateWorkspace", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -215,6 +221,7 @@ def list_workspaces( action="ListWorkspaces", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -315,6 +322,7 @@ def _create_pipeline( action="CreatePipeline", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -375,6 +383,7 @@ def run_pipeline( action="RunPipeline", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -468,6 +477,7 @@ def list_pipeline_runs( action="ListPipelineRuns", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -578,6 +588,7 @@ def list_pipelines( action="ListPipelines", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -693,6 +704,7 @@ def list_pipeline_run_stages_inner( action="ListPipelineRunStagesInner", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, @@ -766,6 +778,7 @@ def get_task_run_log_download_uri( action="GetTaskRunLogDownloadURI", ak=self.volcengine_access_key, sk=self.volcengine_secret_key, + session_token=self.session_token, service=self.service, version=self.version, region=self.region, diff --git a/agentkit/toolkit/volcengine/cr.py b/agentkit/toolkit/volcengine/cr.py index 32ee9eb..3f45b18 100644 --- a/agentkit/toolkit/volcengine/cr.py +++ b/agentkit/toolkit/volcengine/cr.py @@ -37,9 +37,11 @@ def __init__( secret_key: str, region: str | None = None, provider: str | None = None, + session_token: str | None = None, ): self.ak = access_key self.sk = secret_key + self.session_token = session_token config = VolcConfiguration(region=region or None, provider=provider or None) ep = resolve_endpoint( @@ -90,6 +92,7 @@ def _create_instance( action="CreateRegistry", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -149,6 +152,7 @@ def _check_instance(self, instance_name: str) -> str: action="ListRegistries", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -187,6 +191,7 @@ def _create_namespace( action="CreateNamespace", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -239,6 +244,7 @@ def _create_repo( action="CreateRepository", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -273,6 +279,7 @@ def _get_authorization_token(self, instance_name: str): action="GetAuthorizationToken", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -309,6 +316,7 @@ def _get_public_endpoint(self, instance_name: str = DEFAULT_CR_INSTANCE_NAME): action="GetPublicEndpoint", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -337,6 +345,7 @@ def _update_public_endpoint(self, instance_name: str, enabled: bool): action="UpdatePublicEndpoint", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -375,6 +384,7 @@ def _create_endpoint_acl_policies( action="CreateEndpointAclPolicies", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, @@ -405,6 +415,7 @@ def _list_domains(self, instance_name: str = DEFAULT_CR_INSTANCE_NAME): action="ListDomains", ak=self.ak, sk=self.sk, + session_token=self.session_token, service="cr", version=self.version, region=self.region, diff --git a/agentkit/toolkit/volcengine/services/cr_service.py b/agentkit/toolkit/volcengine/services/cr_service.py index 1c4bbf1..14c8d15 100644 --- a/agentkit/toolkit/volcengine/services/cr_service.py +++ b/agentkit/toolkit/volcengine/services/cr_service.py @@ -209,6 +209,7 @@ def _init_client(self, region: Optional[str] = None) -> None: secret_key=creds.secret_key, region=endpoint.region, provider=self.provider, + session_token=creds.session_token, ) # Expose the actual region resolved by VolcConfiguration self.actual_region = endpoint.region diff --git a/agentkit/utils/ve_sign.py b/agentkit/utils/ve_sign.py index 16464d7..2a8e75f 100644 --- a/agentkit/utils/ve_sign.py +++ b/agentkit/utils/ve_sign.py @@ -253,6 +253,7 @@ def request( host=None, content_type=None, scheme=None, + session_token=None, ): # 签名 scope 参数直传（None 时回退到模块级默认，兼容旧直接调用方式） service = Service if service is None else service @@ -294,23 +295,29 @@ def request( "Content-Type": request_param["content_type"], } # 第五步：计算 Signature 签名。 - signed_headers_str = ";".join( - ["content-type", "host", "x-content-sha256", "x-date"] + # STS credentials require X-Security-Token to be part of the signed headers + # (mirrors agentkit/auth/_sigv4.py); static ak/sk callers pass no token and + # keep the original four-header canonical form. + signed_header_pairs = [ + ("content-type", request_param["content_type"]), + ("host", request_param["host"]), + ("x-content-sha256", x_content_sha256), + ("x-date", x_date), + ] + if session_token: + signed_header_pairs.append(("x-security-token", session_token)) + # Canonical headers must be sorted by lowercase header name. + signed_header_pairs.sort(key=lambda kv: kv[0]) + signed_headers_str = ";".join(name for name, _ in signed_header_pairs) + canonical_headers_str = "\n".join( + f"{name}:{value}" for name, value in signed_header_pairs ) - # signed_headers_str = signed_headers_str + ";x-security-token" canonical_request_str = "\n".join( [ request_param["method"].upper(), request_param["path"], norm_query(request_param["query"]), - "\n".join( - [ - "content-type:" + request_param["content_type"], - "host:" + request_param["host"], - "x-content-sha256:" + x_content_sha256, - "x-date:" + x_date, - ] - ), + canonical_headers_str, "", signed_headers_str, x_content_sha256, @@ -347,7 +354,8 @@ def request( ) header = ensure_x_custom_source_header(header) header = {**header, **sign_result} - # header = {**header, **{"X-Security-Token": SessionToken}} + if session_token: + header["X-Security-Token"] = session_token # 第六步：将 Signature 签名写入 HTTP Header 中，并发送 HTTP 请求。 r = _signed_request( method=method, @@ -371,6 +379,7 @@ def ve_request( header: dict | None = None, content_type: str = "application/json", scheme: str = "https", + session_token: str | None = None, ): # 以下参数视服务不同而不同，一个服务内通常是一致的。 # 注意：签名 scope 以参数直传，不再写模块级全局（并发下不同 service/region @@ -395,6 +404,7 @@ def ve_request( host=host, content_type=content_type, scheme=scheme or "https", + session_token=session_token, ) check_error(response_body) return response_body diff --git a/tests/apps/test_agent_server_origin.py b/tests/apps/test_agent_server_origin.py index 954ec05..de09b09 100644 --- a/tests/apps/test_agent_server_origin.py +++ b/tests/apps/test_agent_server_origin.py @@ -146,3 +146,25 @@ def test_add_cors_compat_middleware_splits_literal_and_regex(): assert options["allow_origin_regex"] == "https://.*\\.example\\.com" assert options["allow_methods"] == ["*"] assert options["allow_headers"] == ["*"] + + +def _cors_options(app: FastAPI) -> dict: + [middleware] = app.user_middleware + return getattr(middleware, "options", None) or getattr(middleware, "kwargs", {}) + + +def test_add_cors_compat_middleware_disables_credentials_for_wildcard(): + # A wildcard origin must not be paired with allow_credentials=True, or any + # site could make credentialed cross-origin calls. + app = FastAPI() + add_cors_compat_middleware(app, ["*"]) + assert _cors_options(app)["allow_credentials"] is False + + +def test_add_cors_compat_middleware_keeps_credentials_for_explicit_allowlist(): + app = FastAPI() + add_cors_compat_middleware( + app, + ["https://console.volcengine.com", "regex:https://.*\\.volcengine\\.com"], + ) + assert _cors_options(app)["allow_credentials"] is True diff --git a/tests/toolkit/cli/test_cli_credential.py b/tests/toolkit/cli/test_cli_credential.py index 14f1eb1..32b2b29 100644 --- a/tests/toolkit/cli/test_cli_credential.py +++ b/tests/toolkit/cli/test_cli_credential.py @@ -171,11 +171,66 @@ def delete_inbound_auth_config(self, request): monkeypatch.setattr(client_mod, "AgentkitIdentityClient", _FakeClient) + result = runner.invoke(app, ["delete", "credential", "key-b", "--force"]) + assert result.exit_code == 0, result.output + assert deleted == ["iac-2"] + + +def _single_key_b_client(deleted): + class _FakeClient: + def __init__(self, *args, **kwargs): + pass + + def list_inbound_auth_configs(self, request): + return it.ListInboundAuthConfigsResponse.model_validate( + { + "InboundAuthConfigs": [ + _config("iac-2", "key-b").model_dump(by_alias=True), + ], + "NextToken": "", + } + ) + + def delete_inbound_auth_config(self, request): + deleted.append(request.inbound_auth_config_id) + return it.DeleteInboundAuthConfigResponse.model_validate( + {"InboundAuthConfigId": request.inbound_auth_config_id} + ) + + return _FakeClient + + +def test_delete_credential_non_interactive_proceeds_without_prompt(monkeypatch): + # No TTY (CI / pipe) and no --force: must keep the historical behavior of + # deleting without a prompt, i.e. adding the guard is not a breaking change. + import agentkit.toolkit.cli.cli_delete as cli_delete + + monkeypatch.setattr(cli_delete, "_stdin_is_interactive", lambda: False) + deleted = [] + monkeypatch.setattr( + client_mod, "AgentkitIdentityClient", _single_key_b_client(deleted) + ) + result = runner.invoke(app, ["delete", "credential", "key-b"]) assert result.exit_code == 0, result.output assert deleted == ["iac-2"] +def test_delete_credential_interactive_decline_aborts(monkeypatch): + # With a TTY, declining the prompt must abort before any delete happens. + import agentkit.toolkit.cli.cli_delete as cli_delete + + monkeypatch.setattr(cli_delete, "_stdin_is_interactive", lambda: True) + deleted = [] + monkeypatch.setattr( + client_mod, "AgentkitIdentityClient", _single_key_b_client(deleted) + ) + + result = runner.invoke(app, ["delete", "credential", "key-b"], input="n\n") + assert result.exit_code != 0 + assert deleted == [] + + def test_delete_credential_not_found(monkeypatch): class _FakeClient: def __init__(self, *args, **kwargs): diff --git a/tests/utils/test_ve_sign_signing.py b/tests/utils/test_ve_sign_signing.py index 6675352..d8fc8d2 100644 --- a/tests/utils/test_ve_sign_signing.py +++ b/tests/utils/test_ve_sign_signing.py @@ -121,6 +121,41 @@ def test_request_produces_golden_authorization_header(capture_signed_request): } +def test_request_signs_sts_security_token(capture_signed_request): + """STS credentials must fold X-Security-Token into the signed headers.""" + ve_sign.request( + "POST", + FIXED_DATE, + {"Limit": "2"}, + {}, + "AKTEST", + "SKTEST", + "ListFoo", + json.dumps({"a": 1}), + session_token="STS-abc-123", + **SCOPE, + ) + headers = capture_signed_request["headers"] + + # The token is sent AND covered by the signature (sorted last). + assert headers["X-Security-Token"] == "STS-abc-123" + assert headers["Authorization"] == ( + "HMAC-SHA256 Credential=AKTEST/20260102/cn-test/test-svc/request, " + "SignedHeaders=content-type;host;x-content-sha256;x-date;x-security-token, " + "Signature=a60cb3db3fea2df93ddb8a0cc149bee51d9a50c30a7d35a53c3b9d2c511f9c57" + ) + + +def test_request_without_token_omits_security_token(capture_signed_request): + """Static ak/sk callers keep the original four-header canonical form.""" + ve_sign.request( + "POST", FIXED_DATE, {}, {}, "AK", "SK", "ActA", "", **SCOPE + ) + headers = capture_signed_request["headers"] + assert "X-Security-Token" not in headers + assert "x-security-token" not in headers["Authorization"] + + def test_request_scope_is_per_call_not_global(capture_signed_request): """Two calls with different scopes must not bleed into each other.""" ve_sign.request( From f899154048e47cddbb45d39c10f5af2313a552d5 Mon Sep 17 00:00:00 2001 From: "liyi.ly" Date: Tue, 7 Jul 2026 09:36:21 +0800 Subject: [PATCH 2/2] fix: tighten credential hygiene for logs and on-disk registry - redact: add secret_key, security_token and signature to the field list so the repo's own config key names and presigned-URL HMACs are masked; add regression tests (secrets masked, trace-id/git-sha/uuid preserved). - harness deploy: write harness.json (which stores the runtime API key in plaintext) with mode 0600 instead of default umask. - cli deploy: mask the API key in terminal output (full value stays in harness.json) and note the file must not be committed. - gitignore: ignore harness.json. --- .gitignore | 2 + agentkit/toolkit/cli/cli_deploy.py | 9 ++++- agentkit/toolkit/harness/deploy.py | 9 ++++- agentkit/utils/redact.py | 7 +++- tests/utils/test_redact.py | 61 ++++++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/utils/test_redact.py diff --git a/.gitignore b/.gitignore index 448cd17..6becb64 100644 --- a/.gitignore +++ b/.gitignore @@ -174,6 +174,8 @@ docs/node_modules/ test-agents/* tos_doc_example agentkit*.yaml +# harness deploy registry — contains the runtime API key in plaintext +harness.json Dockerfile Dockerfile-base local_build.py diff --git a/agentkit/toolkit/cli/cli_deploy.py b/agentkit/toolkit/cli/cli_deploy.py index 2fc45d1..0b4cce8 100644 --- a/agentkit/toolkit/cli/cli_deploy.py +++ b/agentkit/toolkit/cli/cli_deploy.py @@ -183,8 +183,13 @@ def _deploy_harness( console.print(f"[green]Runtime id: {meta['runtime_id']}[/green]") console.print(f"[green]Endpoint: {endpoint or '(see AgentKit console)'}[/green]") if meta.get("runtime_apikey"): - console.print(f"[green]API key: {meta['runtime_apikey']}[/green]") + from agentkit.utils.redact import mask + + # Avoid leaking the full key into terminal scrollback / CI logs; the + # complete value is persisted (0600) in harness.json below. + console.print(f"[green]API key: {mask(meta['runtime_apikey'])}[/green]") if endpoint: console.print( - f"[green]Recorded in {(Path.cwd() / 'harness.json')}[/green]" + f"[green]Recorded in {(Path.cwd() / 'harness.json')} " + "(contains the API key — do not commit)[/green]" ) diff --git a/agentkit/toolkit/harness/deploy.py b/agentkit/toolkit/harness/deploy.py index 9ad925f..332adf5 100644 --- a/agentkit/toolkit/harness/deploy.py +++ b/agentkit/toolkit/harness/deploy.py @@ -137,7 +137,14 @@ def _record_harness( } else: data[name] = {"url": url, "key": key or "", "runtime_id": runtime_id} - path.write_text(json.dumps(data, indent=2, ensure_ascii=False)) + payload = json.dumps(data, indent=2, ensure_ascii=False) + # harness.json stores the runtime API key in plaintext, so keep it private + # (0600). os.open applies the mode only when creating the file; chmod after + # covers the case where an older, world-readable harness.json already exists. + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(payload) + os.chmod(path, 0o600) return path diff --git a/agentkit/utils/redact.py b/agentkit/utils/redact.py index f83f432..e6e63a4 100644 --- a/agentkit/utils/redact.py +++ b/agentkit/utils/redact.py @@ -37,10 +37,13 @@ # OTel trace_id (32), git sha (40), sha256 digest (64). Labeled hex secrets # are still caught by _FIELD regardless of shape. _HEX_ID_LENGTHS = frozenset({32, 40, 64}) -# Explicit secret-bearing query/JSON/header fields. +# Explicit secret-bearing query/JSON/header fields. secret_access_key is listed +# before secret_key so the longer name wins at a shared position; signature +# covers presigned-URL HMACs whose 64-hex value _redact_opaque otherwise spares. _FIELD = re.compile( r"(?i)(\"?(?:access_token|refresh_token|id_token|client_secret|secret_access_key" - r"|secretkey|accesskeyid|accesskey|sessiontoken|session_token|authorization" + r"|secret_key|secretkey|accesskeyid|accesskey|sessiontoken|session_token" + r"|security_token|signature|authorization" r"|apikey|api_key|token|password)\"?\s*[:=]\s*\"?(?:bearer\s+)?)" r"([^\"&\s,}]+)" ) diff --git a/tests/utils/test_redact.py b/tests/utils/test_redact.py new file mode 100644 index 0000000..cdbe718 --- /dev/null +++ b/tests/utils/test_redact.py @@ -0,0 +1,61 @@ +# Copyright (c) 2025 Beijing Volcano Engine Technology Co., Ltd. and/or its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Regression coverage for :func:`agentkit.utils.redact.redact`. + +The field list is driven by the secret key names this repo actually uses in its +config and error messages (e.g. ``volcengine.secret_key``), so a rename there +should be mirrored here. +""" + +import pytest + +from agentkit.utils.redact import redact + +# Each string carries a secret whose value must not survive redaction. +SECRET_CASES = [ + "secret_key=AKLTaBcSecretValue123456", + "volcengine.secret_key: myLongSecretKeyValue0099", + "secret_access_key=SKverylongsecretvalue0099", + "session_token=abcLONGsessiontoken123456", + 'X-Security-Token: STSverylongtokenvalue123XYZ', + "authorization: Bearer eyJhbGciOiJIUzI1NiJ9.payloadsegment.signaturesegment", + "api_key=sk-abcdef0123456789abcdef", + "Signature=" + "a1b2c3d4" * 8, # 64-hex presigned-URL HMAC +] + +# Well-known identifier shapes that must be preserved so logs stay debuggable. +SURVIVOR_CASES = [ + "trace_id 4bf92f3577b34da6a3ce929d0e0e4736", # 32-hex OTel trace id + "commit 5eaa7e2abc1234567890abcdef1234567890abcd", # 40-hex git sha + "request_id 550e8400-e29b-41d4-a716-446655440000", # uuid +] + + +@pytest.mark.parametrize("text", SECRET_CASES) +def test_redact_masks_secret_fields(text): + assert "***" in redact(text) + + +@pytest.mark.parametrize("text", SECRET_CASES) +def test_redact_drops_the_secret_value(text): + # The literal secret value (everything after the delimiter) must be gone. + delimiter = ":" if ":" in text.split("=", 1)[0] else "=" + value = text.split(delimiter, 1)[1].strip().removeprefix("Bearer ") + assert value not in redact(text) + + +@pytest.mark.parametrize("text", SURVIVOR_CASES) +def test_redact_preserves_identifiers(text): + assert redact(text) == text