Repository navigation
Conversation
|
Prepared issuer follow-through is pushed separately at The component runs the real validator on bounded, inert inputs, binds the All 98 local tests pass: 27 issuer/caller cases plus the existing 71 reader, This does not enable a workflow or policy exception. Protected bootstrap, |
rmcdaniel
left a comment
rmcdaniel
left a comment
There was a problem hiding this comment.
Independent maintainer source review of exact ed2334a72976044b0d7aa37f6235de35cae71e13 (tree 4bd8fb6ba11f68748523fd1622bb76d6ff40a915), in progress. One concrete hash-boundary finding is below.
I reran 47 existing exact-source constructed controls for input capture, bounded read transport and caller context. All pass. The temporary source snapshot was retired; no dependency installation, live App request, Check Run write, credential/settings change or protected-source edit was made. This is not Security-owner acknowledgment or live enforcement qualification. Review of the remaining boundaries and required live controls continues.
| policy["workflow_trust"]["pull_request_target_exceptions"] == [PROTECTED_POLICY_EXCEPTION] | ||
| and target_name == PROTECTED_POLICY_EXCEPTION["target"] | ||
| and Path(workflow_path).parts[-3:] == Path(PROTECTED_POLICY_EXCEPTION["path"]).parts | ||
| and hashlib.sha256(source.encode("utf-8")).hexdigest() == PROTECTED_POLICY_EXCEPTION["sha256"] |
There was a problem hiding this comment.
[P2] Preserve raw workflow bytes before checking the exception hash
validate_local_action_references reads files with Path.read_text, which normalizes CRLF and lone CR to LF. This helper therefore hashes normalized text rather than the collected Git blob bytes.
A constructed control using the exact helper and protected workflow shows the effect: reviewed raw SHA-256 642d204ea939ca5eba7ef51be9e4ae56a35177413caabe849315086d3d9d2d31; after changing LF to CRLF, raw SHA-256 54b9cf6a3b3da2ac1337cfeab6976d53dd5815f8e3ccb84bacf3d82bb9178673; the helper still returns True after the current read path. No repository bytes or App state were changed.
This control changes line endings only; it does not demonstrate a semantic or executable workflow change. It does contradict the documented exact-byte/content-hash exception. Preserve original bytes through hashing (or preserve newlines during decoding) and add CRLF/lone-CR file controls before relying on that guarantee.
There was a problem hiding this comment.
Prepared and pushed in exact combined source 619c0d595aed0fe9214ed6b1aa0155216bb3bc59, whole de9d1564ea7b7f6e683343a7c36ae91e35733644. Both file-level newline mutations fail before the byte-read correction; the unchanged LF and ordinary workflow controls pass. The alias-work counterfactual also fails before its correction. All 110 focused controls now pass. The initial cycle fixture expected acceptance of a value the existing BaseLoader already refuses; its corrected refusal control preserves the parser and is not claimed as a second product defect. Protected workflow SHA-256 remains unchanged. Fresh whole-source qualification and the named follow-up review remain pending; no App/Environment or live enforcement claim.
rmcdaniel
left a comment
rmcdaniel
left a comment
There was a problem hiding this comment.
Additional immutable ed2334a7 source review: the API transport has byte/count/time bounds, but the YAML graph walk has a separate availability risk. A bounded constructed control is described below.
With the exact source and the hash-verified locked PyYAML 6.0.2 wheel's pure-Python BaseLoader, all 79 existing input, reader and issuer tests pass locally. This is not a native ABI/install qualification; the existing hosted amd64/arm64 wheel receipts cover that separate scope. The temporary source/parser snapshot was retired. No live App/API write or credential/settings/source change was made.
| candidate = Path(temporary) / "data" | ||
| write_inputs(files, candidate) | ||
| try: | ||
| actions = check(policy, candidate / ".github/workflows", "github-control-plane") |
There was a problem hiding this comment.
Residual availability risk: raw byte limits do not bound YAML graph traversal
This call reaches _workflow_action_references, whose recursive visitor follows aliases repeatedly without a visited-node or work budget. A 434-byte, 14-level acyclic alias fixture parsed by the locked PyYAML BaseLoader exceeded 10,000 visits before an instrumented stop; a 206-byte, four-level control took 98. Increasing the alias depth multiplies work while remaining far below the file/transfer bounds. No long-running/hanging probe was performed.
The 120-second API deadline is checked around requests, not while this visitor runs. The outer 10-minute job timeout is the current final time limit. This is resource amplification, not a demonstrated credential or executable-code bypass.
Before enabling the untrusted-input trigger, bound logical YAML work (including other recursive walks), or explicitly assess and retain the outer-timeout risk. A small alias fan-out control belongs with the constructed tests; raw size and JSON-depth controls do not cover it.
There was a problem hiding this comment.
Prepared and pushed in exact combined source 619c0d595aed0fe9214ed6b1aa0155216bb3bc59, whole de9d1564ea7b7f6e683343a7c36ae91e35733644. Both file-level newline mutations fail before the byte-read correction; the unchanged LF and ordinary workflow controls pass. The alias-work counterfactual also fails before its correction. All 110 focused controls now pass. The initial cycle fixture expected acceptance of a value the existing BaseLoader already refuses; its corrected refusal control preserves the parser and is not claimed as a second product defect. Protected workflow SHA-256 remains unchanged. Fresh whole-source qualification and the named follow-up review remain pending; no App/Environment or live enforcement claim.
…kflow # Conflicts: # .github/workflows/sonar.yml
rmcdaniel
left a comment
rmcdaniel
left a comment
There was a problem hiding this comment.
Completed independent maintainer source review of exact ed2334a72976044b0d7aa37f6235de35cae71e13, tree 4bd8fb6ba11f68748523fd1622bb76d6ff40a915. This review does not qualify a later combined revision or attribute Security-owner approval.
The exact-byte finding remains a source defect in this snapshot. The YAML graph observation is a separate availability risk: byte/JSON transport bounds do not bound recursive alias work, whose current final limit is the outer job timeout. Neither control demonstrated an executable workflow change or credential bypass.
I traced the data-only reader and issuer boundaries: fixed public pilot and repository ID; exact event/base/head identity; canonical regular-file inputs and nonexecutable temporary materialization outside the import path; fixed HTTPS metadata endpoints with explicit digest-checked roots; separate read and Checks-write client roles; protected workflow/source markers; App/name/head/check-ID verification; and main/PR rechecks immediately before completion. The App key is supplied to the pinned token action, not the reader/issuer program. The token action manifest at 64ab729a... recognizes the requested repository and Checks-write scope and uses Node 24. Actual credential scope and denial remain platform properties to prove.
Verification: all 79 existing input/reader/issuer controls passed in an exact-source temporary snapshot using the hash-verified locked PyYAML wheel's pure-Python BaseLoader; this is not a native ABI claim. Existing run 39907499738 checks out c6b4625f..., whose tree independently matches 4bd8fb6b...; it records 193 shared tests, both native wheel jobs, and a read-only API report bound to this head and policy SHA-256 550d11be0ae511644f2b05b4927d214e19ca0fbabe899e20c1d80ab8fface5f7. Both lock hashes match the official PyPI CPython 3.13 glibc amd64/arm64 wheels. No live App/check-write probe was performed. All temporary review material is retired.
Live adoption still needs these distinct controls:
- Establish the actual pilot-only App installation, effective Checks-write/Metadata-read token scope and protected Environment credential path. Pair negative fork/same-repository/unprotected-context denial with a genuine protected positive control; missing credentials everywhere cannot prove denial. Caller environment markers and constructed App responses are insufficient.
- Verify the credentialed token action's effective trust selection and failure behavior. The read-only API exercise does not prove the App create/revoke path or its credential denial.
- Qualify the actual required-check rule with the expected App ID and exact
Protected workflow policyname, preserving existing protections and strictness. A same-name Actions/wrong-App check must not satisfy it; safe correct-App/current-head success must. The ordinary policy audit checks context names/strictness, not this new App binding. - Exercise stale head/main/policy, malformed input, interrupted/ambiguous completion and same-head rerun behavior. In particular, validation precedes
issuer.begin; an early refusal emits no new App check and does not revoke an existing success on that same SHA. Whether prior success can remain qualifying after trusted-policy/base changes needs explicit design/live qualification. This is an unqualified residual, not a demonstrated bypass.
Bootstrap order should keep source review/qualification, protected-main bootstrap, account/Environment preparation, live issuer controls and actual ruleset-enforcement qualification distinguishable. A bootstrap merge is a separately reviewed trust-root action; candidate CI cannot authorize it. Require the final combined source's fresh checks and a named follow-up review, preserving the workflow bytes/hash and existing CI/Sonar module entries. This gate validates workflow inputs, not arbitrary replacements of its own policy, reader, issuer or bootstrap code. Those trust-root changes retain independent review.
rmcdaniel
left a comment
rmcdaniel
left a comment
There was a problem hiding this comment.
Additional bootstrap-contract pass on immutable ed2334a72976044b0d7aa37f6235de35cae71e13.
[P2] Bind the preflight working directory as well as its command text.
The job-shape check accepts jobs.action-policy.defaults.run.working-directory; verify_workflow_source also drops workflow-level defaults before calling that helper. Both the locked install and validator commands use relative .central-action-policy/... paths. Setting either default to candidate-controlled therefore makes them select candidate-controlled/.central-action-policy/..., which the candidate can supply, while the reviewed checkout still places protected source at $GITHUB_WORKSPACE/.central-action-policy.
Constructed control through the exact committed validate_local_action_references(policy, directory, "server"), using the existing preflight fixture and locked bootstrap/install: the baseline, added job default, and added workflow default all pass with the same three approved external actions. No candidate command, install, live workflow, or credential operation was executed. Parser: hash-verified PyYAML 6.0.2 wheel Python modules; temporary snapshot removed.
GitHub documents that these defaults apply to run steps (default working-directory semantics); checkout documents its workspace-relative path.
Please bind the effective directory for both commands, or reject both levels of directory override, and add failing-before controls for each. This is a protected-source/preflight contract defect, not evidence of App credential exposure or a live enforcement bypass. The prior issuer/reader findings and required live controls remain unchanged.
rmcdaniel
left a comment
rmcdaniel
left a comment
There was a problem hiding this comment.
Named follow-up review: 619c0d595aed0fe9214ed6b1aa0155216bb3bc59, tree de9d1564ea7b7f6e683343a7c36ae91e35733644.
The raw-byte correction resolves the reviewed line-ending defect. Independent failing-before controls on ed2334a7 produce CRLF, lone-CR and alias-work failures; the corrected source passes 126 focused tests, including the Composer contract module. The reader, caller, issuer and protected workflow are byte-identical to ed2334a7; protected workflow SHA-256 remains 642d204ea939ca5eba7ef51be9e4ae56a35177413caabe849315086d3d9d2d31. The CI/Sonar integration retains both protected and metrics modules.
Two follow-up findings:
-
[P2] Bound secret-context traversal too. The corrected action-reference walker visits shared containers once, but
_string_valuesstill recursively expands them while the full workflow scanner checks secret references. A 637-byte candidate YAML graph with fourteen binary alias levels passes the corrected action-reference walk, then exceeds 10,000 calls in this remaining traversal. An instrumented budget stopped at 10,001; no long-running or memory-exhaustion probe was performed. The availability/work-amplification risk therefore remains in the full entry point. Preserve the parser's existing recursive-node refusal and cover the full scan, rather than only_workflow_action_references. -
[P2] Apply effective-directory reasoning to Composer helper inspection.
_composer_step_sourcealways inspectsroot / name;_composer_job_sinksdoes not supply the actual directory. Through the exact full validator, a harmless rootscripts/request.shand a network-request helper atalternate/scripts/request.share accepted without a bootstrap/verifier when the samescripts/request.shcommand uses step, job-default or workflow-defaultworking-directory: alternate. Acd alternateinside the run script is also accepted. Moving that network request into the inspected root helper correctly refuses it. No helper was executed. Bind or conservatively refuse directory contexts that the source proof cannot resolve. GitHub's default-directory semantics apply to these run steps.
The original preflight-directory finding in review 5627276128 remains present. The separate privileged-dispatch _step_invokes proof already rejects shell/directory overrides at step, job and workflow levels. The exact-byte protected workflow also prevents such candidate changes to that workflow.
Verification: local CPython 3.12.3 with hash-verified PyYAML 6.0.2 Python modules, not native ABI qualification. Existing CI 39941988083 passes 203 shared tests and two wheel controls on each native runner. All three jobs checked out 48df8d44cbc3f8578c66e463637a60cf188e800a, whose tree equals this review's tree. All nine head checks pass; direct Sonar gate is OK with zero open/confirmed PR issues. Temporary snapshots/dependencies/raw are retired.
This is an independent maintainer source review, not Security-owner acknowledgment or live enforcement approval. App setup, Environment denial, real issuer binding and the other live controls in review 5627156061 remain pending. Further corrections need fresh qualification and a separately identified review.
|
Implementation component for integration and independent review:
Validation: Only the policy implementation and two test modules changed. Reader, caller, issuer, protected workflow, policy JSON, parser lock, schema and CI/Sonar lists are byte-identical to the base; protected workflow SHA-256 remains This component needs independent review and fresh combined-source qualification in this PR. App setup and actual Environment/issuer enforcement remain pending; no activation follows from these source tests. |
|
Additional pre-existing preflight execution-context finding, carried through the scoped cwd/graph component: inherited custom shells are still accepted. A source-only control using the exact committed GitHub documents that the shell template receives the temporary run-script path at This is another preflight contract defect, not a demonstrated App credential or live issuer bypass. The independently reviewed cwd/graph component remains unchanged. Bind the effective shell to reviewed semantics, or refuse unreviewed inherited templates, with failing-before controls and fresh combined qualification. Implementation/design disposition remains with this PR's owner. |
|
Focused shell-binding follow-up: The install, validator, result-gate step and Composer relative-helper proof bind the effective shell using the most-specific run setting. Ordinary runner defaults and reviewed built-in The actionable S5778 finding Validation: 146 focused tests pass across policy, Composer and all three protected-policy modules. Three selected shell controls fail against the reviewed predecessor with six failure entries; all pass after correction. Local CPython 3.12.3 / hash-verified PyYAML 6.0.2 Python modules, not native ABI qualification. Independent log readback of 7a5 run 39945932877 confirms 216 retained tests passed, then the read-only client refused because protected main moved. That is not a current-source qualification pass. This follow-up needs independent integration review and fresh checks together with current main. No live App/Environment/issuer result or activation is claimed. |
|
Focused result-gate refusal component: The reviewed result-test step must preserve failure. The guard uses the existing One focused control produces four failure entries against This adds one guard and one regression in two files. Protected workflow, reader/check/issuer, policy thresholds and exception scope remain untouched. Integrate the shell and this result-gate component together with qualified main, then freshly qualify that exact combined source. No live bypass, App readiness or activation is claimed. |
|
Closed execution-context component: Bounded AST-only controls confirmed that exact The component closes the dedicated preflight job/default/action-step shapes. Its caller-supplied environment is a closed reviewed set, reusing the existing allowed-environment check. Ordinary Action metadata, the known GitHub-host condition, explicit false/default failure handling, locked bootstrap and command paths remain supported. 155 focused controls pass across policy, Composer and the three protected-policy modules; Only the policy implementation and its focused test module changed. The same nine protected/config files are byte-identical, including protected workflow SHA-256 The earlier |
|
Public caller-context inventory for the 60c follow-up, read at 12:46–12:47 UTC on 2026-10-23. Each main ref was resolved first, then the selected workflow was fetched by immutable commit and parsed with PyYAML BaseLoader as data. These are source observations, not new runtime or enforcement results. The policy target selection is the exact f825 snapshot; comparison is f825 versus component Across all ten public caller preflights above, workflow/job The Server caller is an existing bootstrap compatibility gap, not a 60c regression. Its five-step preflight still runs The ordinary result gate remains a narrower structural assertion. In separate bounded AST-only fixtures, adding gate-local No additional implementation was made for this inventory. The 155 focused controls and f825's earlier passing gates retain their exact snapshot scope. |
|
|
Independent combined-source qualification is complete for A fresh disposable CPython 3.13.16 / pip 26.2.2 environment with the hash-locked native PyYAML 6.0.2 wheel passes 155 focused controls plus both actual dependency-installer cases (157 total). The protected workflow remains SHA-256 The pinned caller inventory confirms no new environment/default incompatibility in this component. Server's existing unlocked five-step bootstrap fails both versions and still needs its own adoption migration. Ordinary caller gate execution is outside the structural preflight proof. This remains a draft, unactivated source result. Independent issuer App/environment and actual live allow/deny, wrong-issuer, fork, stale/malformed, ambiguous-write and required-rule qualification remain the activation criteria. No settings or repository enforcement are changed, and there is no substitute based on a spoofable caller check context. |
|
Reader/issuer/bootstrap follow-up review of exact Scope is the central input reader, read client, issuer and prepared workflow. I authored the separate policy component
Fresh independent verification on CPython 3.12.3: all 27 input-reader controls pass, zero skips. Exact reader blob Required live/integration controls remain:
No additional concrete source defect found in these reviewed components. The App/account setup and live acceptance remain pending. No protected file, setting, credential or enforcement state changed during this review. |
The candidate workflow must not be able to authorize its own CI policy result. This draft separates protected-source policy validation from the candidate checkout and binds the eventual required status to an independent issuer.
The implementation uses the protected policy source, isolated hash-locked parser dependencies, bounded YAML/alias traversal and exact reviewed preflight actions. It verifies effective shell, working directory and Composer network-call guards, and refuses skipped or ignored preflight execution. Caller-supplied preflight containers, services, strategy and startup-hook environments are outside the reviewed shape. The ordinary caller result-gate job is not a general execution sandbox or an independent authority boundary.
Exact current source:
d4fe8e08d9e7dfdd2c1b24e477e59573385c30f0, whole tree0398da440f63bf176211ae3766b9d6ae49fd2ac6, incorporating qualified maince4a7caand the final preflight-context component. The protected workflow remains SHA-256642d204ea939ca5eba7ef51be9e4ae56a35177413caabe849315086d3d9d2d31; reader, check, issuer, parser lock and policy thresholds retain their reviewed bytes.Validation: the exact combined source passes 157 focused controls on CPython 3.13.16, pip 26.2.2 and the hash-locked native PyYAML 6.0.2 wheel: 155 policy/reader/input/issuer/Composer cases plus both real dependency-installer controls. The installer ignores working-directory pip shadows and rejects a tampered wheel hash. The initial local runner attempts failed because a non-executable temporary mount could not load the native extension and the installer test requires a disposable venv; those were setup failures, not source passes. The final disposable venv run passes without weakening assertions. Fresh GitHub checks qualify this head independently.
A pinned main-caller inventory finds no new environment/default incompatibility from the final component. The existing Server legacy five-step/unlocked bootstrap fails both the preceding and new checker, so its migration remains a separate adoption prerequisite. Other callers retain their actual reviewed results.
Not activated. Organization required status context plus integration ID does not bind a workflow identity. The independent issuer App/environment, live allow/deny and fork, stale, malformed, wrong-issuer, ambiguous-write and required-rule controls remain necessary before activation. Structural source tests do not substitute for those live controls. No account setting, required rule or repository enforcement is changed by this PR.
The earlier combined source passed its hosted source controls but its API check failed after protected main changed. That original remains failed; passing results are not transferred across heads. Related: #237.