From 36677431b24db5134949912c78fab6cd4cf82a71 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Wed, 4 Nov 2026 05:25:58 +0800 Subject: [PATCH] fix: Skip PAPER identity checks for disabled targets --- .github/workflows/sync-cloud-run-env.yml | 74 ++++++++++--------- docs/runtime-target-manifest.md | 16 ++-- tests/test_paper_deploy_wif_binding.py | 89 ++++++++++++++++++++++- tests/test_sync_cloud_run_env_workflow.sh | 8 ++ 4 files changed, 143 insertions(+), 44 deletions(-) diff --git a/.github/workflows/sync-cloud-run-env.yml b/.github/workflows/sync-cloud-run-env.yml index d9571152..280cc6e1 100644 --- a/.github/workflows/sync-cloud-run-env.yml +++ b/.github/workflows/sync-cloud-run-env.yml @@ -802,43 +802,15 @@ jobs: STRATEGY_PLUGIN_ALERT_PUSH_ACCESS_TOKEN: ${{ secrets.STRATEGY_PLUGIN_ALERT_PUSH_ACCESS_TOKEN }} STRATEGY_PLUGIN_ALERT_TELEGRAM_BOT_TOKEN: ${{ secrets.TG_TOKEN }} steps: - - name: Mask WIF identity components - if: matrix.target.environment == 'longbridge-paper' + - name: Check whether Cloud Run automation is enabled + id: config + # Select the target without exposing inherited identities or reading PAPER Secrets. env: - # Suppress inherited identity fields while the runner prints this step. GCP_PROJECT_ID: "" - GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} GCP_RUNTIME_SERVICE_ACCOUNT: "" GCP_SCHEDULER_SERVICE_ACCOUNT: "" - GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT: ${{ secrets.GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT }} - run: | - set -euo pipefail - account="$GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT" - if ! [[ "$account" =~ ^[a-z0-9-]+@[a-z0-9-]+\.iam\.gserviceaccount\.com$ ]]; then - printf '%s\n' '::error::Required Secret GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT is missing or malformed' - exit 1 - fi - provider="${GCP_WORKLOAD_IDENTITY_PROVIDER:-}" - if ! [[ "$provider" =~ ^projects/([0-9]+)/locations/global/workloadIdentityPools/([a-z0-9-]+)/providers/([a-z0-9-]+)$ ]]; then - printf '%s\n' '::error::Required Secret GCP_WORKLOAD_IDENTITY_PROVIDER is missing or malformed' - exit 1 - fi - project_number="${BASH_REMATCH[1]}" - pool_name="${BASH_REMATCH[2]}" - provider_name="${BASH_REMATCH[3]}" - account_name="${account%%@*}" - project="${account#*@}" - project="${project%.iam.gserviceaccount.com}" - printf '::add-mask::%s\n' "$account_name" - printf '::add-mask::%s\n' "$project" - printf '::add-mask::%s\n' "$project_number" - printf '::add-mask::%s\n' "$pool_name" - printf '::add-mask::%s\n' "$provider_name" - printf '::add-mask::%s\n' "$provider" - printf 'GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT=%s\n' "$account" >> "$GITHUB_ENV" - printf 'GCP_WORKLOAD_IDENTITY_PROVIDER=%s\n' "$provider" >> "$GITHUB_ENV" - - name: Check whether Cloud Run automation is enabled - id: config + GCP_WORKLOAD_IDENTITY_PROVIDER: "" + GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT: "" run: | set -euo pipefail @@ -953,6 +925,42 @@ jobs: write_github_output "enabled=true" + - name: Mask WIF identity components + if: matrix.target.environment == 'longbridge-paper' && steps.config.outputs.enabled == 'true' + env: + # Suppress inherited identity fields while the runner prints this step. + GCP_PROJECT_ID: "" + GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }} + GCP_RUNTIME_SERVICE_ACCOUNT: "" + GCP_SCHEDULER_SERVICE_ACCOUNT: "" + GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT: ${{ secrets.GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT }} + run: | + set -euo pipefail + account="$GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT" + if ! [[ "$account" =~ ^[a-z0-9-]+@[a-z0-9-]+\.iam\.gserviceaccount\.com$ ]]; then + printf '%s\n' '::error::Required Secret GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT is missing or malformed' + exit 1 + fi + provider="${GCP_WORKLOAD_IDENTITY_PROVIDER:-}" + if ! [[ "$provider" =~ ^projects/([0-9]+)/locations/global/workloadIdentityPools/([a-z0-9-]+)/providers/([a-z0-9-]+)$ ]]; then + printf '%s\n' '::error::Required Secret GCP_WORKLOAD_IDENTITY_PROVIDER is missing or malformed' + exit 1 + fi + project_number="${BASH_REMATCH[1]}" + pool_name="${BASH_REMATCH[2]}" + provider_name="${BASH_REMATCH[3]}" + account_name="${account%%@*}" + project="${account#*@}" + project="${project%.iam.gserviceaccount.com}" + printf '::add-mask::%s\n' "$account_name" + printf '::add-mask::%s\n' "$project" + printf '::add-mask::%s\n' "$project_number" + printf '::add-mask::%s\n' "$pool_name" + printf '::add-mask::%s\n' "$provider_name" + printf '::add-mask::%s\n' "$provider" + printf 'GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT=%s\n' "$account" >> "$GITHUB_ENV" + printf 'GCP_WORKLOAD_IDENTITY_PROVIDER=%s\n' "$provider" >> "$GITHUB_ENV" + - name: Checkout repository if: steps.config.outputs.enabled == 'true' uses: actions/checkout@v6 diff --git a/docs/runtime-target-manifest.md b/docs/runtime-target-manifest.md index c0b02092..c20f317c 100644 --- a/docs/runtime-target-manifest.md +++ b/docs/runtime-target-manifest.md @@ -152,13 +152,15 @@ Cloud workflows read `GCP_PROJECT_ID`, `GCP_WORKLOAD_IDENTITY_PROVIDER`, and `GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT` from their existing GitHub environment variables. PAPER jobs in `sync-cloud-run-env.yml` and the PAPER inspection workflow read the WIF service account from the protected `longbridge-paper` Environment -Secret `GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT` instead. Their first step validates -the Secret and masks its derived account and project identifiers before exporting -the identity for later authentication. The deployment masking step blanks inherited -cloud identity fields so the runner cannot print them before the component masks -are registered; later steps retain their configured values. Missing or malformed -Secrets fail closed -without falling back to the variable. Other deployment targets keep their +Secret `GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT` instead. Before exporting the +identity for later authentication, they validate the Secret and mask its derived +account and project identifiers. Legacy deployment first selects the target and +checks automation enablement with inherited cloud identity fields blanked and +without reading PAPER WIF Secrets. Only a selected, enabled PAPER deployment runs +the masking step, which also blanks inherited cloud identity fields. This prevents +the runner from printing them before the component masks are registered; later +steps retain their configured values. Missing or malformed Secrets for an enabled +PAPER deployment fail closed without falling back to the variable. Other deployment targets keep their existing variable source. Workflows that compare runtime or Scheduler identities additionally require `GCP_RUNTIME_SERVICE_ACCOUNT` or `GCP_SCHEDULER_SERVICE_ACCOUNT`. Configure diff --git a/tests/test_paper_deploy_wif_binding.py b/tests/test_paper_deploy_wif_binding.py index 3007670d..3f298d1a 100644 --- a/tests/test_paper_deploy_wif_binding.py +++ b/tests/test_paper_deploy_wif_binding.py @@ -12,6 +12,11 @@ PROVIDER_MASKS = ["123456789012", "fake-pool", "fake-provider", PROVIDER] +def mask_step(job): + return next(step for step in job_section(job).split(" steps:\n", 1)[1].split("\n - ") + if "name: Mask WIF identity components" in step) + + def masked(text, masks): for value in sorted(masks, key=len, reverse=True): text = text.replace(value, "***") @@ -33,7 +38,9 @@ def test_paper_binding_never_expands_variable_identity_before_masks(job, conditi config, steps = section.split(" steps:\n", 1) inverse = condition.replace(" == ", " != ") assert f"GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT: ${{{{ {inverse} && vars.GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT || '' }}}}" in config - first = steps.split("\n - ", 1)[0] + first = mask_step(job) + if job == "image-only-no-traffic": + assert steps.split("\n - ", 1)[0] == first assert "name: Mask WIF identity components" in first assert f"if: {condition}" in first assert "GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT: ${{ secrets.GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT }}" in first @@ -57,7 +64,7 @@ def test_paper_binding_never_expands_variable_identity_before_masks(job, conditi pytest.param("fake-agent@fake-project.iam.gserviceaccount.com", id="valid"), ]) def test_paper_mask_step_blocks_invalid_secret_and_masks_before_export(tmp_path, job, identity): - first = job_section(job).split(" steps:\n", 1)[1].split("\n - ", 1)[0] + first = mask_step(job) assert "name: Mask WIF identity components" in first script = textwrap.dedent(first.split(" run: |\n", 1)[1]) exported = tmp_path / "env" @@ -88,7 +95,7 @@ def test_paper_mask_step_blocks_invalid_secret_and_masks_before_export(tmp_path, @pytest.mark.parametrize("job", ["image-only-no-traffic", "sync"]) def test_next_step_environment_and_auth_input_mask_provider_components(tmp_path, job): deployment = job_section(job) - first = deployment.split(" steps:\n", 1)[1].split("\n - ", 1)[0] + first = mask_step(job) script = textwrap.dedent(first.split(" run: |\n", 1)[1]) exported = tmp_path / "env" # Automatic masks exist only for values actually bound through secrets.*. @@ -142,7 +149,7 @@ def test_next_step_environment_and_auth_input_mask_provider_components(tmp_path, pytest.param(PROVIDER + "\rvalue", id="carriage-return"), ]) def test_invalid_provider_blocks_export_and_next_step(tmp_path, job, provider): - first = job_section(job).split(" steps:\n", 1)[1].split("\n - ", 1)[0] + first = mask_step(job) script = textwrap.dedent(first.split(" run: |\n", 1)[1]) exported = tmp_path / "env" marker = tmp_path / "next-step" @@ -158,3 +165,77 @@ def test_invalid_provider_blocks_export_and_next_step(tmp_path, job, provider): assert result.stdout == "::error::Required Secret GCP_WORKLOAD_IDENTITY_PROVIDER is missing or malformed\n" assert not exported.exists() assert not marker.exists() + + +IDENTITY_KEYS = ("GCP_PROJECT_ID", "GCP_RUNTIME_SERVICE_ACCOUNT", + "GCP_SCHEDULER_SERVICE_ACCOUNT", "GCP_WORKLOAD_IDENTITY_PROVIDER", + "GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT") + + +def test_selection_precedes_secret_access_without_identity_disclosure(): + section = job_section("sync") + first, second, *_ = section.split(" steps:\n", 1)[1].split("\n - ") + assert "name: Check whether Cloud Run automation is enabled" in first + assert "secrets." not in first + for key in IDENTITY_KEYS: + assert f' {key}: ""' in first + assert "name: Mask WIF identity components" in second + assert "if: matrix.target.environment == 'longbridge-paper' && steps.config.outputs.enabled == 'true'" in second + assert section.index("name: Mask WIF identity components") < section.index("name: Checkout repository") + + +@pytest.mark.parametrize("target,deploy,sync,identity,provider,expected_enabled,expected_code", [ + pytest.param("hk-verify", "true", "true", "", "", False, 0, id="hk-only-missing"), + pytest.param("hk-verify", "true", "true", "malformed", "malformed", False, 0, id="excluded-malformed"), + pytest.param("configured", "false", "false", "", "", False, 0, id="disabled-missing"), + pytest.param("paper-command-verify", "false", "false", "malformed", "malformed", False, 0, id="disabled-malformed"), + pytest.param("paper-command-verify", "true", "true", "", PROVIDER, True, 1, id="paper-missing-account"), + pytest.param("paper-command-verify", "true", "true", "malformed", PROVIDER, True, 1, id="paper-malformed-account"), + pytest.param("paper-command-verify", "true", "true", ACCOUNT, "", True, 1, id="paper-missing-provider"), + pytest.param("paper-command-verify", "true", "true", ACCOUNT, "malformed", True, 1, id="paper-malformed-provider"), + pytest.param("paper-command-verify", "true", "true", ACCOUNT, PROVIDER, True, 0, id="paper-valid"), +]) +def test_paper_selection_and_validation_sequence(tmp_path, target, deploy, sync, + identity, provider, expected_enabled, + expected_code): + # Execute the actual first two workflow scripts in runner order. Resolve + # Secret bindings only when their step condition selects this PAPER row. + output = tmp_path / "output" + exported = tmp_path / "env" + environment = { + **os.environ, "GITHUB_EVENT_NAME": "workflow_dispatch", + "WORKFLOW_TARGET": target, "DEPLOYMENT_LABEL": "PAPER", + "ENABLE_MAIN_PUSH_CLOUD_RUN_AUTOMATION": "false", + "QSL_ENABLE_CLOUD_RUN_AUTOMATION": "false", + "ENABLE_GITHUB_CLOUD_RUN_DEPLOY": deploy, + "ENABLE_GITHUB_ENV_SYNC": sync, "INPUT_DEPLOY_IMAGE": deploy, + "INPUT_SYNC_ENV": sync, "INPUT_SHIFT_TRAFFIC_TO_COMMIT": "false", + "INPUT_SCHEDULER_ENABLED_ACTION": "preserve", + "GITHUB_OUTPUT": str(output), "GITHUB_ENV": str(exported), + } + outputs = {} + secret_reads = 0 + returncode = 0 + for step in job_section("sync").split(" steps:\n", 1)[1].split("\n - ")[:2]: + if "name: Mask WIF identity components" in step: + condition = step.split(" if: ", 1)[1].splitlines()[0] + condition = condition.replace("matrix.target.environment", repr("longbridge-paper")) + condition = condition.replace("steps.config.outputs.enabled", repr(outputs.get("enabled", ""))) + if not eval(condition.replace("&&", "and"), {"__builtins__": {}}, {}): + continue + secret_reads += 1 + environment.update(GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT=identity, + GCP_WORKLOAD_IDENTITY_PROVIDER=provider) + script = textwrap.dedent(step.split(" run: |\n", 1)[1]) + result = subprocess.run(["bash", "-c", script], env=environment, + capture_output=True, text=True, timeout=10, check=False) + returncode = result.returncode + if returncode: + assert "::error::Required Secret" in result.stdout + break + if output.exists(): + outputs.update(line.split("=", 1) for line in output.read_text().splitlines()) + assert returncode == expected_code + assert secret_reads == int(expected_enabled) + assert outputs.get("enabled") == str(expected_enabled).lower() + assert exported.exists() == (expected_enabled and expected_code == 0) diff --git a/tests/test_sync_cloud_run_env_workflow.sh b/tests/test_sync_cloud_run_env_workflow.sh index 92927925..428d43c2 100644 --- a/tests/test_sync_cloud_run_env_workflow.sh +++ b/tests/test_sync_cloud_run_env_workflow.sh @@ -438,6 +438,14 @@ assert "render_runtime_target_matrix.py --profile sync --github-output" in workf assert "matrix: ${{ fromJSON(needs.resolve-matrix.outputs.matrix) }}" in workflow legacy = workflow.split("\n sync:\n", 1)[1].split("\n cleanup-shared-monitor:\n", 1)[0] cleanup = workflow.split("\n cleanup-shared-monitor:\n", 1)[1] +selection, masking, *_ = legacy.split(" steps:\n", 1)[1].split("\n - ") +assert "name: Check whether Cloud Run automation is enabled" in selection +assert "secrets." not in selection +for key in ("GCP_PROJECT_ID", "GCP_RUNTIME_SERVICE_ACCOUNT", "GCP_SCHEDULER_SERVICE_ACCOUNT", + "GCP_WORKLOAD_IDENTITY_PROVIDER", "GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT"): + assert f' {key}: ""' in selection +assert "name: Mask WIF identity components" in masking +assert "if: matrix.target.environment == 'longbridge-paper' && steps.config.outputs.enabled == 'true'" in masking for section in (legacy, cleanup): assert "inputs.deployment_mode == 'legacy'" in section.split(" steps:", 1)[0] assert "needs: resolve-matrix" in legacy.split(" steps:", 1)[0]