From 50bcbb9f824131d3aae9f95c6753b8b4f060db69 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:47:36 +0800 Subject: [PATCH] fix: inspect remaining gateway connections with protected target identity Co-Authored-By: Codex --- .../read-only-vm-metadata-diagnostic.yml | 100 ++++++++++++------ tests/test_gateway_workflow_resolve.cjs | 68 +++++++++++- tests/test_inspect_gateway_connections.sh | 5 +- .../test_read_only_vm_metadata_diagnostic.sh | 2 +- 4 files changed, 137 insertions(+), 38 deletions(-) diff --git a/.github/workflows/read-only-vm-metadata-diagnostic.yml b/.github/workflows/read-only-vm-metadata-diagnostic.yml index 41c0b3e..590d397 100644 --- a/.github/workflows/read-only-vm-metadata-diagnostic.yml +++ b/.github/workflows/read-only-vm-metadata-diagnostic.yml @@ -75,8 +75,8 @@ jobs: core.setFailed("Passive inspection requires protected current-gateway matching"); return; } - if (remainingGateways && (!matchMode || inspectConnections || inspectSshPolicy || inspectSshKeyBinding)) { - core.setFailed("Remaining gateway metadata inspection requires protected match-only mode"); + if (remainingGateways && (!matchMode || inspectSshPolicy || inspectSshKeyBinding)) { + core.setFailed("Remaining gateway inspection requires protected metadata or passive-connection mode"); return; } if (inspectSshKeyBinding && !inspectSshPolicy) { @@ -124,6 +124,40 @@ jobs: core.setFailed("Protected gateway target inventory is invalid"); return; } + if (remainingGateways) { + const matchedIndex = parseProtectedGatewayIndex(process.env.MATCHED_INDEX); + if (matchedIndex === undefined) { + core.setFailed("Protected matched target index is unavailable"); + return; + } + const remainingTargets = targets + .map((target, target_index) => ({ target, target_index })) + .filter(({ target_index }) => target_index !== matchedIndex); + const invalidTarget = remainingTargets.some(({ target }) => { + if (!/^[a-z][a-z0-9-]{4,28}[a-z0-9]$/.test(target.gcp_project_id) || + !/^[a-z]([-a-z0-9]*[a-z0-9])?$/.test(target.gce_instance_name) || + !/^[a-z]+(?:-[a-z]+)*[0-9]+-[a-z]$/.test(target.gce_zone)) return true; + if (!inspectConnections) return false; + const secretProject = target.gcp_secret_project_id || target.gcp_project_id; + return !/^[a-z][a-z0-9-]{4,28}[a-z0-9]$/.test(secretProject) || + !/^[A-Za-z_][A-Za-z0-9_.-]*$/.test(target.gce_user || "") || + !/^[A-Za-z0-9_-]+$/.test(target.ssh_private_key_secret_name || "") || + !/^[A-Za-z0-9][A-Za-z0-9_.-]*$/.test(target.container_name || "") || + !["paper", "live"].includes(target.mode); + }); + if (invalidTarget) { + core.setFailed(inspectConnections + ? "Protected remaining target connection configuration is incomplete" + : "Protected remaining target metadata configuration is incomplete"); + return; + } + core.setOutput("matrix", JSON.stringify({include: remainingTargets.map(({ target_index }) => ({ + target_index, + inspect_connections: inspectConnections, + inspect_ssh_policy: false, + }))})); + return; + } if (inspectConnections || inspectSshPolicy || inspectSshKeyBinding) { const selectedIndex = parseProtectedGatewayIndex(process.env.MATCHED_INDEX); if (selectedIndex === undefined) { @@ -148,25 +182,6 @@ jobs: core.setOutput("matrix", JSON.stringify({include: [{target_index: selectedIndex, inspect_connections: inspectConnections, inspect_ssh_policy: inspectSshPolicy}]})); return; } - if (remainingGateways) { - const matchedIndex = parseProtectedGatewayIndex(process.env.MATCHED_INDEX); - if (matchedIndex === undefined) { - core.setFailed("Protected matched target index is unavailable"); - return; - } - const remainingTargets = targets.filter((_, targetIndex) => targetIndex !== matchedIndex); - if (remainingTargets.some((target) => - !/^[a-z][a-z0-9-]{4,28}[a-z0-9]$/.test(target.gcp_project_id) || - !/^[a-z]([-a-z0-9]*[a-z0-9])?$/.test(target.gce_instance_name) || - !/^[a-z]+(?:-[a-z]+)*[0-9]+-[a-z]$/.test(target.gce_zone))) { - core.setFailed("Protected remaining target metadata configuration is incomplete"); - return; - } - core.setOutput("matrix", JSON.stringify({include: targets - .map((_, target_index) => ({target_index})) - .filter(({target_index}) => target_index !== matchedIndex)})); - return; - } core.setOutput("matrix", JSON.stringify({include: targets.map((_, target_index) => ({target_index}))})); return; } @@ -223,8 +238,8 @@ jobs: const inspectSshPolicy = process.env.INSPECT_SSH_POLICY === "true"; const inspectSshKeyBinding = process.env.INSPECT_SSH_KEY_BINDING === "true"; const remainingGateways = process.env.REMAINING_GATEWAYS === "true"; - if (remainingGateways && (!matchMode || inspectConnections || inspectSshPolicy || inspectSshKeyBinding)) { - core.setFailed("Remaining gateway metadata inspection requires protected match-only mode"); + if (remainingGateways && (!matchMode || inspectSshPolicy || inspectSshKeyBinding)) { + core.setFailed("Remaining gateway inspection requires protected metadata or passive-connection mode"); return; } const raw = matchMode ? process.env.MATCH_TARGETS_JSON : process.env.LEGACY_TARGETS_JSON; @@ -262,17 +277,19 @@ jobs: core.setFailed("Protected gateway target inventory is invalid"); return; } - const matchedIndex = (inspectConnections || inspectSshPolicy || inspectSshKeyBinding) ? parseProtectedGatewayIndex(process.env.MATCHED_INDEX) : undefined; - if ((inspectConnections || inspectSshPolicy || inspectSshKeyBinding) && (matchedIndex === undefined || matchedIndex !== targetIndex)) { + const matchedIndex = (inspectConnections || inspectSshPolicy || inspectSshKeyBinding || remainingGateways) ? parseProtectedGatewayIndex(process.env.MATCHED_INDEX) : undefined; + if ((inspectConnections || inspectSshPolicy || inspectSshKeyBinding) && !remainingGateways && + (matchedIndex === undefined || matchedIndex !== targetIndex)) { core.setFailed("Protected matched target index is unavailable"); return; } - if (remainingGateways) { - const matchedIndex = parseProtectedGatewayIndex(process.env.MATCHED_INDEX); - if (matchedIndex === undefined || matchedIndex === targetIndex) { - core.setFailed("Resolved gateway target is not an eligible remaining target"); - return; - } + if (remainingGateways && matchedIndex === undefined) { + core.setFailed("Protected matched target index is unavailable"); + return; + } + if (remainingGateways && matchedIndex === targetIndex) { + core.setFailed("Resolved gateway target is not an eligible remaining target"); + return; } target = targets[targetIndex]; if (remainingGateways && @@ -282,6 +299,17 @@ jobs: core.setFailed("Protected remaining target metadata configuration is incomplete"); return; } + if (remainingGateways && inspectConnections) { + const secretProject = target.gcp_secret_project_id || target.gcp_project_id; + if (!/^[a-z][a-z0-9-]{4,28}[a-z0-9]$/.test(String(secretProject || "")) || + !/^[A-Za-z_][A-Za-z0-9_.-]*$/.test(String(target.gce_user || "")) || + !/^[A-Za-z0-9_-]+$/.test(String(target.ssh_private_key_secret_name || "")) || + !/^[A-Za-z0-9][A-Za-z0-9_.-]*$/.test(String(target.container_name || "")) || + !["paper", "live"].includes(target.mode)) { + core.setFailed("Protected remaining target connection configuration is incomplete"); + return; + } + } } else { target = targets[targetIndex]; const crypto = require("crypto"); @@ -463,6 +491,9 @@ jobs: python3 scripts/match_gateway_metadata.py \ --metadata-file "${metadata_file}" --target-index "${TARGET_INDEX}" \ --verify-target-identity; then + if [ "${INSPECT_CONNECTIONS}" = "true" ]; then + echo "gateway_connection_target_verified=true" >> "${GITHUB_OUTPUT}" + fi exit 0 fi exit 1 @@ -485,6 +516,9 @@ jobs: fi exit 1 fi + if [ "${INSPECT_CONNECTIONS}" = "true" ] && [ "${match_status}" = "match" ]; then + echo "gateway_connection_target_verified=true" >> "${GITHUB_OUTPUT}" + fi exit "${match_exit}" fi VM_METADATA_FILE="${metadata_file}" python3 - <<'PY' @@ -501,7 +535,7 @@ jobs: PY - name: Read SSH key into a restricted temporary file - if: ${{ inputs.inspect_connections && steps.metadata_check.outputs.gateway_ip_match_status == 'match' }} + if: ${{ inputs.inspect_connections && steps.metadata_check.outputs.gateway_connection_target_verified == 'true' }} env: SSH_PRIVATE_KEY_SECRET_NAME: ${{ steps.metadata.outputs.ssh_private_key_secret_name }} GCP_PROJECT_ID: ${{ steps.metadata.outputs.gcp_secret_project_id }} @@ -677,7 +711,7 @@ jobs: fi - name: Inspect Gateway connections through one IAP SSH session - if: ${{ inputs.inspect_connections && steps.metadata_check.outputs.gateway_ip_match_status == 'match' }} + if: ${{ inputs.inspect_connections && steps.metadata_check.outputs.gateway_connection_target_verified == 'true' }} env: GCP_PROJECT_ID: ${{ steps.metadata.outputs.gcp_project_id }} TARGET_INDEX: ${{ matrix.target_index }} diff --git a/tests/test_gateway_workflow_resolve.cjs b/tests/test_gateway_workflow_resolve.cjs index 29d244a..8c66da3 100644 --- a/tests/test_gateway_workflow_resolve.cjs +++ b/tests/test_gateway_workflow_resolve.cjs @@ -151,15 +151,44 @@ for (const matchedIndex of ['', 'bad', '{"target_index":4}', '1']) { for (const invalidMode of [ { MATCH_CURRENT_GATEWAY: 'false', INSPECT_CONNECTIONS: 'false', INSPECT_SSH_POLICY: 'false', REMAINING_GATEWAYS: 'true' }, - { MATCH_CURRENT_GATEWAY: 'true', INSPECT_CONNECTIONS: 'true', INSPECT_SSH_POLICY: 'false', REMAINING_GATEWAYS: 'true' }, { MATCH_CURRENT_GATEWAY: 'true', INSPECT_CONNECTIONS: 'false', INSPECT_SSH_POLICY: 'true', REMAINING_GATEWAYS: 'true' }, { MATCH_CURRENT_GATEWAY: 'true', INSPECT_CONNECTIONS: 'false', INSPECT_SSH_POLICY: 'false', INSPECT_SSH_KEY_BINDING: 'true', REMAINING_GATEWAYS: 'true' }, ]) { const remaining = runResolve({ ...invalidMode, MATCH_TARGETS_JSON: JSON.stringify(targets), MATCHED_INDEX: '{"target_index":2}' }); - assert.equal(remaining.failure, 'Remaining gateway metadata inspection requires protected match-only mode'); + assert.equal(remaining.failure, 'Remaining gateway inspection requires protected metadata or passive-connection mode'); assert.equal(remaining.outputs.matrix, undefined); } +const remainingConnections = runResolve({ + MATCH_CURRENT_GATEWAY: 'true', + INSPECT_CONNECTIONS: 'true', + INSPECT_SSH_POLICY: 'false', + INSPECT_SSH_KEY_BINDING: 'false', + REMAINING_GATEWAYS: 'true', + MATCH_TARGETS_JSON: JSON.stringify(targets), + MATCHED_INDEX: '{"target_index":2}', +}); +assert.equal(remainingConnections.failure, null); +assert.deepEqual(JSON.parse(remainingConnections.outputs.matrix).include, [ + { target_index: 0, inspect_connections: true, inspect_ssh_policy: false }, + { target_index: 1, inspect_connections: true, inspect_ssh_policy: false }, + { target_index: 3, inspect_connections: true, inspect_ssh_policy: false }, +]); + +const missingRemainingConnectionConfig = runResolve({ + MATCH_CURRENT_GATEWAY: 'true', + INSPECT_CONNECTIONS: 'true', + INSPECT_SSH_POLICY: 'false', + INSPECT_SSH_KEY_BINDING: 'false', + REMAINING_GATEWAYS: 'true', + MATCH_TARGETS_JSON: JSON.stringify(targets.map((target, index) => index === 1 + ? (({ ssh_private_key_secret_name, ...rest }) => rest)(target) + : target)), + MATCHED_INDEX: '{"target_index":2}', +}); +assert.equal(missingRemainingConnectionConfig.failure, 'Protected remaining target connection configuration is incomplete'); +assert.equal(missingRemainingConnectionConfig.outputs.matrix, undefined); + const remainingDiagnose = runMetadataResolve({ MATCH_CURRENT_GATEWAY: 'true', INSPECT_CONNECTIONS: 'false', @@ -192,7 +221,36 @@ const combinedDiagnose = runMetadataResolve({ MATCHED_INDEX: '{"target_index":2}', TARGET_INDEX: '1', }); -assert.equal(combinedDiagnose.failure, 'Remaining gateway metadata inspection requires protected match-only mode'); +assert.equal(combinedDiagnose.failure, null); +assert.equal(combinedDiagnose.exports.TARGET_INDEX, '1'); +assert.equal(combinedDiagnose.exports.GCE_USER, 'mock-user'); +assert.equal(combinedDiagnose.outputs.ssh_private_key_secret_name, 'mock-ssh-key'); +assert.equal(combinedDiagnose.outputs.gcp_secret_project_id, 'mock-project-1'); +assert.equal(combinedDiagnose.outputs.gateway_ip_expected_host, undefined); +assert.ok(combinedDiagnose.secrets.includes('mock-ssh-key')); + +const combinedMatchedDiagnose = runMetadataResolve({ + MATCH_CURRENT_GATEWAY: 'true', + INSPECT_CONNECTIONS: 'true', + INSPECT_SSH_POLICY: 'false', + REMAINING_GATEWAYS: 'true', + MATCH_TARGETS_JSON: JSON.stringify(targets), + MATCHED_INDEX: '{"target_index":2}', + TARGET_INDEX: '2', +}); +assert.equal(combinedMatchedDiagnose.failure, 'Resolved gateway target is not an eligible remaining target'); +assert.equal(combinedMatchedDiagnose.outputs.ssh_private_key_secret_name, undefined); + +const invalidRemainingMetadataMode = runMetadataResolve({ + MATCH_CURRENT_GATEWAY: 'true', + INSPECT_CONNECTIONS: 'false', + INSPECT_SSH_POLICY: 'true', + REMAINING_GATEWAYS: 'true', + MATCH_TARGETS_JSON: JSON.stringify(targets), + MATCHED_INDEX: '{"target_index":2}', + TARGET_INDEX: '1', +}); +assert.equal(invalidRemainingMetadataMode.failure, 'Remaining gateway inspection requires protected metadata or passive-connection mode'); const inspect = runResolve({ MATCH_CURRENT_GATEWAY: 'true', @@ -241,6 +299,10 @@ const badBinding = runResolve({ }); assert.equal(badBinding.failure, 'Passive inspection requires protected current-gateway matching'); +assert.match(workflow, /gateway_connection_target_verified=true/); +assert.match(workflow, /if: \$\{\{ inputs\.inspect_connections && steps\.metadata_check\.outputs\.gateway_connection_target_verified == 'true' \}\}/); +assert.doesNotMatch(workflow, /inputs\.remaining_gateways && steps\.metadata_check\.outputs\.gateway_ip_match_status/); + const policyBadBinding = runResolve({ MATCH_CURRENT_GATEWAY: 'false', INSPECT_CONNECTIONS: 'false', diff --git a/tests/test_inspect_gateway_connections.sh b/tests/test_inspect_gateway_connections.sh index a6a15f7..9ba6656 100644 --- a/tests/test_inspect_gateway_connections.sh +++ b/tests/test_inspect_gateway_connections.sh @@ -82,7 +82,10 @@ grep -Fq '(inputs.inspect_connections || inputs.inspect_ssh_policy || inputs.rem grep -Fq 'parseProtectedGatewayIndex(process.env.MATCHED_INDEX)' "${workflow_file}" grep -Fq 'Passive inspection requires protected current-gateway matching' "${workflow_file}" grep -Fq 'Protected matched target connection configuration is incomplete' "${workflow_file}" -grep -Fq 'steps.metadata_check.outputs.gateway_ip_match_status == '\''match'\''' "${workflow_file}" +grep -Fq 'steps.metadata_check.outputs.gateway_connection_target_verified == '\''true'\''' "${workflow_file}" +grep -Fq 'gateway_connection_target_verified=true' "${workflow_file}" +grep -Fq 'Protected remaining target connection configuration is incomplete' "${workflow_file}" +grep -Fq 'remainingGateways && (!matchMode || inspectSshPolicy || inspectSshKeyBinding)' "${workflow_file}" grep -Fq 'gcloud compute start-iap-tunnel' "${workflow_file}" grep -Fq 'ss -H -tn state established sport = ":${api_port}"' "${script_file}" grep -Fq 'gcloud secrets versions access latest' "${workflow_file}" diff --git a/tests/test_read_only_vm_metadata_diagnostic.sh b/tests/test_read_only_vm_metadata_diagnostic.sh index 5cb1fea..db9af36 100755 --- a/tests/test_read_only_vm_metadata_diagnostic.sh +++ b/tests/test_read_only_vm_metadata_diagnostic.sh @@ -48,7 +48,7 @@ grep -Fq "LEGACY_TARGETS_JSON: \${{ !inputs.match_current_gateway && vars.IB_GAT grep -Fq "IB_GATEWAY_EXPECTED_HOST: \${{ inputs.match_current_gateway && !inputs.remaining_gateways && secrets.IB_GATEWAY_EXPECTED_HOST || '' }}" "$workflow_file" grep -Fq 'fail-fast: ${{ !inputs.remaining_gateways }}' "$workflow_file" grep -Fq 'max-parallel: ${{ inputs.remaining_gateways && 3 || 1 }}' "$workflow_file" -grep -Fq 'Remaining gateway metadata inspection requires protected match-only mode' "$workflow_file" +grep -Fq 'Remaining gateway inspection requires protected metadata or passive-connection mode' "$workflow_file" grep -Fq 'Resolved gateway target is not an eligible remaining target' "$workflow_file" grep -Fq -- '--verify-target-identity' "$workflow_file" grep -Fq 'GATEWAY_VM_DIAGNOSTIC_TARGET_INDEX=' "$workflow_file"