diff --git a/.github/workflows/grant-paper-executor-permissions.yml b/.github/workflows/grant-paper-executor-permissions.yml index b733172..dc79a06 100644 --- a/.github/workflows/grant-paper-executor-permissions.yml +++ b/.github/workflows/grant-paper-executor-permissions.yml @@ -64,27 +64,55 @@ jobs: 'PAPER_IAM_BOOTSTRAP_SERVICE_ACCOUNT', 'PAPER_IAM_BOOTSTRAP_WIF_PROVIDER' ]; const mappings = []; - try { - for (const name of names) { - let data; + for (const name of names) { + let scope = 'environment'; + let category = 'transport_error'; + const read = async (route, args) => { + let response; try { - ({data} = await github.request( - 'GET /repos/{owner}/{repo}/environments/{environment_name}/variables/{name}', - {...context.repo, environment_name: 'longbridge-paper', name})); + response = await github.request(route, args); } catch (error) { - if (error.status !== 404) throw error; - ({data} = await github.request( - 'GET /repos/{owner}/{repo}/actions/variables/{name}', - {...context.repo, name})); + const status = error?.status; + if (status === 401 || status === 403 || status === 429) { + category = `http_${status}`; + } else if (status === 404) { + category = scope === 'repository' ? 'http_404_both' : 'other_http'; + } else if (Number.isInteger(status) && status >= 500 && status <= 599) { + category = 'http_5xx'; + } else if (Number.isInteger(status)) { + category = 'other_http'; + } else { + category = 'transport_error'; + } + // Only an Environment HTTP 404 permits the same-name fallback. + if (scope === 'environment' && status === 404) return null; + throw new Error('Mapping lookup failed'); + } + const data = response?.data; + if (!data || typeof data.value !== 'string') { + category = 'malformed_response'; + throw new Error('Invalid mapping response'); } - if (typeof data.value !== 'string' || !data.value.trim()) { - throw new Error('Missing mapping'); + if (!data.value.trim()) { + category = 'blank_value'; + throw new Error('Empty mapping'); + } + return data; + }; + try { + let data = await read( + 'GET /repos/{owner}/{repo}/environments/{environment_name}/variables/{name}', + {...context.repo, environment_name: 'longbridge-paper', name}); + if (data === null) { + scope = 'repository'; + data = await read('GET /repos/{owner}/{repo}/actions/variables/{name}', + {...context.repo, name}); } core.setSecret(data.value); mappings.push([name, data.value]); + } catch { + throw new Error(`Protected mapping read failed: name=${name} scope=${scope} category=${category}; stop before authentication`); } - } catch { - throw new Error('Protected mapping read failed; stop before authentication'); } for (const [name, value] of mappings) core.exportVariable(name, value); - name: Verify Environment approval protection remains configured diff --git a/docs/iam/paper-scheduler-get.md b/docs/iam/paper-scheduler-get.md index 045a870..7b08db9 100644 --- a/docs/iam/paper-scheduler-get.md +++ b/docs/iam/paper-scheduler-get.md @@ -225,6 +225,16 @@ workflow definition. Variable API access by the workflow token is a separate prerequisite and is not established by local tests; an inaccessible mapping blocks the route and does not authorize provisioning a token or changing access. +A mapping-read failure reports only the failing fixed variable name, lookup +scope (`environment` or `repository`) and an allowlisted category: `http_401`, +`http_403`, `http_404_both`, `http_429`, `http_5xx`, `other_http`, `blank_value`, +`malformed_response` or `transport_error`. `http_404_both` means that both +same-name lookups returned HTTP 404. A non-string or absent value is a malformed +response; an empty or whitespace-only string is a blank value. No raw exception, +URL, response, headers or mapping value is included. If any read fails, no +mapping is exported, including previously read values. This diagnostic does not +prove a permission repair or authorize a retry, grant, or PAPER start. + Role permission responses are checked for exact membership and cardinality, independent of order. Fresh role snapshots normalize permission ordering while retaining etag and every other field for concurrent-drift detection. diff --git a/tests/test_paper_executor_grant.py b/tests/test_paper_executor_grant.py index e9dd403..884e125 100644 --- a/tests/test_paper_executor_grant.py +++ b/tests/test_paper_executor_grant.py @@ -255,3 +255,72 @@ def test_mapping_masks_precede_all_step_details(): .catch(() => process.exit(1)); """.replace("SCRIPT", json.dumps(script)).replace("KEYS", json.dumps(mappings)) subprocess.run(["node", "-e", harness], check=True, capture_output=True) + + +@pytest.mark.parametrize("index", range(6)) +@pytest.mark.parametrize("scope", ["environment", "repository"]) +@pytest.mark.parametrize("failure,category", [ + (401, "http_401"), (403, "http_403"), (429, "http_429"), + (500, "http_5xx"), (599, "http_5xx"), (400, "other_http"), + (302, "other_http"), (600, "other_http"), ("403", "transport_error"), + ("transport", "transport_error"), ("null_error", "transport_error"), + ("blank", "blank_value"), ("null", "malformed_response"), + ("missing", "malformed_response"), ("wrong_type", "malformed_response"), + ("null_response", "malformed_response"), + (404, "http_404_both"), +]) +def test_mapping_failure_diagnostic_is_allowlisted_and_exports_nothing(index, scope, failure, category): + # Execute the actual inline action offline, including failures after partial success. + workflow = (Path(__file__).parents[1] / + ".github/workflows/grant-paper-executor-permissions.yml").read_text() + first = workflow.split(" - name:")[1] + script = textwrap.dedent(first.split(" script: |\n", 1)[1]) + harness = r""" +const assert = require('node:assert/strict'); +const keys = ['GCP_PROJECT_ID', 'PAPER_SCHEDULER_GET_PROJECT_ID', + 'GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT', 'PAPER_IAM_GRANT_RECIPIENT', + 'PAPER_IAM_BOOTSTRAP_SERVICE_ACCOUNT', 'PAPER_IAM_BOOTSTRAP_WIF_PROVIDER']; +const [index, scope, failure, category] = CASE; +const calls = [], masks = [], exports = []; +const core = {setSecret: value => masks.push(value), + exportVariable: (...args) => exports.push(args)}; +const github = {request: async (route, args) => { + assert.equal(exports.length, 0); + const actualScope = route.includes('/environments/') ? 'environment' : 'repository'; + calls.push([args.name, actualScope]); + if (args.name !== keys[index]) return {data: {value: 'fake-' + args.name}}; + if (actualScope === 'environment' && (scope === 'repository' || failure === 404)) { + throw {status: 404, message: 'fake-private-payload'}; + } + if (failure === 'blank') return {data: {value: ' '}}; + if (failure === 'null') return {data: null}; + if (failure === 'missing') return {}; + if (failure === 'wrong_type') return {data: {value: 17}}; + if (failure === 'null_response') return null; + if (failure === 'null_error') throw null; + const error = new Error('fake-private-payload'); + error.response = {data: 'fake-private-payload', headers: {'fake': 'fake-private-payload'}}; + error.request = {url: 'fake-private-payload'}; + if (failure !== 'transport') error.status = failure; + throw error; +}}; +const context = {repo: {owner: 'fake-owner', repo: 'fake-repo'}}; +const run = new Function('github', 'core', 'context', 'return (async () => {' + SCRIPT + '})()'); +(async () => { + await assert.rejects(run(github, core, context), error => { + const expectedScope = failure === 404 ? 'repository' : scope; + assert.equal(error.message, 'Protected mapping read failed: name=' + keys[index] + + ' scope=' + expectedScope + ' category=' + category + '; stop before authentication'); + assert.equal(error.cause, undefined); + return true; + }); + assert.deepEqual(exports, []); + assert.equal(masks.length, index); + const expected = keys.slice(0, index).map(name => [name, 'environment']); + expected.push([keys[index], 'environment']); + if (scope === 'repository' || failure === 404) expected.push([keys[index], 'repository']); + assert.deepEqual(calls, expected); +})().catch(error => { console.error(error); process.exit(1); }); +""".replace("SCRIPT", json.dumps(script)).replace("CASE", json.dumps([index, scope, failure, category])) + result = subprocess.run(["node", "-e", harness], capture_output=True, text=True) + assert result.returncode == 0, result.stderr