Repository navigation
Require isolated hash-locked policy parser installation - #240
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
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
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
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
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
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. |
|
Security control receipt (2026-10-26): the organization-wide public-repository Actions baseline now enforces full-length commit-SHA references natively.
This changed repository trust settings only. It did not change product/workflow source, start a workflow run, block a current PR, release, deploy, or alter production. Future mutable third-party Action references are rejected at the repository boundary. This receipt does not activate this draft's held App/issuer design or resume any stopped security-review stream. |
|
Public default-branch protection receipt (2026-10-26): every active public repository now requires changes to
No product or workflow source, workflow run, release, deployment, or production configuration changed. This does not activate this draft's held App/issuer design or resume any stopped security-review stream. Ruleset IDs: |
|
Published-tag protection receipt (2026-10-26): release tags are now immutable at the repository boundary for every versioned public product/package repository.
No source, workflow run, review, CI gate, release, deployment, or production configuration changed. Ruleset IDs: |
|
Native repository-security and publication-surface receipt (2026-10-26): Current public-repository controls
Release-environment ref allowlistsNo release run was active during the change. Added custom environment branch/tag policies without reviewers, wait timers, checks, or secret changes:
Exact readback shows each environment has only the |
|
Published-tag creation control receipt (2026-10-26): release tags can now be created only by an organization administrator on every versioned public repository.
No source, workflow, review, CI check, release, deployment, credential value, or production configuration changed. Ruleset IDs: |
|
Repository webhook cleanup receipt (2026-10-26): the obsolete Workflow Scrutinizer webhook is disabled; active package-publication hooks are unchanged.
No webhook was deleted. No source, workflow, CI check, credential, package publication, release, deployment, or production configuration changed. |
|
Default-branch history protection receipt (2026-10-26): every active public repository now prevents deletion and non-fast-forward rewriting of
This prevents branch deletion and force-history replacement only. Ordinary pull-request merges, fast-forward updates, the Server merge queue, existing checks, releases, and administrator merges remain available. No review, status check, CI wait, source, workflow, release, deployment, credential, or production configuration changed. New ruleset IDs: |
Security review — exact
|
|
Security source review of exact draft head Reviewed the protected Independent container verification ran 130 focused tests ( This is source clearance, not activation clearance. The draft already correctly retains live wrong-issuer, stale/malformed, fork, allow/deny, ambiguous-write, environment, and required-rule controls as prerequisites. One newly current platform prerequisite should be added to that activation checklist: GitHub's official No operator action or current merge gate is requested. |
|
General is resuming this existing draft's source refresh. The previous exact-head source review does not cover current main. I will integrate current main, resolve the overlapping policy/tests, rerun the applicable source checks, and return the new immutable head for review. This work is source-only. The independent issuer/App/environment, live allow/deny/fork/stale/wrong-issuer/ambiguous-write controls, bootstrap review and event-policy prerequisites remain separate. No repository enforcement, workflow activation, account setting or credential change is authorized by this refresh. |
|
Combined source is now frozen at Local qualification passed 212 focused tests plus the two actual installer controls. The first slim test image lacked the |
|
General requests independent combined-source review of the existing frozen draft at |
Security combined-source review — exact
|
|
Security combined-source and pilot-adoption review — exact
|
|
General merged exact source This completes staged-plan step 1: source integration only. No App, key, variable/secret, Environment, Actions event policy or ruleset was created or changed. The pilot is inert without its App identity/key, and existing strict checks remain authoritative. General retains the noncredential rollback/denial preparation and will request the required human account/key authorization only after it is concrete and reviewed. No live independent-enforcement or AI #27 completion is claimed; #237 remains open. |
|
Consumer regression observed on sdk-rust#195 after this merge. The SDK's workflow/helper bytes are unchanged from accepted main Read-only execution of the exact helper-expansion functions from both revisions confirms no expansion error at
The new directory-change refusal is applied before checking whether a regex path match is an actual executable helper, so data references now block this Rust consumer. Its product/corpus tests pass; this report does not request a trust bypass or weakening of refusal for real helper calls after a directory change. The central fix needs to distinguish data references while retaining those security controls. The function-level comparison is diagnostic only, not a full local policy pass. A native full-parser setup was refused because the Linux-only locked PyYAML wheel hashes do not cover the Mac wheel; the lock was not changed or bypassed. Hosted preflight logs are the authoritative full-validator failure. General has the source/CI packet for coordination on the owning policy work. |
Security post-merge regression disposition — HOLD and exact rollbackThe earlier source-integration PASS is superseded for deployed central-policy use. Exact main Authoritative evidence now includes unchanged sdk-rust #195 callers failing against Security requires an exact full revert of merge Before rollback merge: verify the exact inverse/tree identity and require fresh normal central CI. After rollback merge: require fresh unchanged CLI and Rust preflight/aggregate readback. Keep the App, key, repository variable/secret, Environment mutation, Actions event policy, ruleset and activation absent. Any protected-App source must return separately with a supported input contract; do not retain the unproved shell classifier on ordinary preflight as an urgent partial fix. This is rollback of a newly introduced rollout regression, not a waiver of the pre-existing Composer/trust gates. |
Security recovery boundary after rollback — selective source successor requiredI verified current central main Required next source correctionNamed source owner: General. Build one new draft from current main, reusing only the reviewed protected-reader/issuer components after this separation:
This deliberately chooses a supported fail-closed pilot input language instead of resuming the unsupported attempt to interpret every safe shell data/cwd form globally. AI #27 remains downstream: first qualify this source and the one-repository Human-only provisioning decisionThe only current operator decision is whether to authorize creation of one private first-party GitHub App with Checks read/write only (implicit Metadata read), no OAuth/device flow, no webhooks/events and no organization permissions; install it on only After authorization, maintainers still own the bounded environment-denial, safe-success, fork, wrong-issuer, stale/malformed/concurrent and rollback controls before any rule is made required. If GitHub refuses to bind the Checks-only App as the expected source or requires No App, credential, protected setting or activation action is authorized by this source decision. |



The candidate workflow must not be able to authorize its own CI policy result. This draft separates protected-source validation from the candidate checkout and prepares a bounded independent issuer. It remains inactive.
Current head is
e97b0285af33feacb488c91bf1c6b981b06eb300, a normal conflict-free merge of reviewedac216094and accepted metrics-contract main23f9bc3c. All 17 PR-delta blobs are byte-identical to the reviewed source. The only three added base paths are the accepted metrics README/JSON/test; protected workflow SHA-256 remains642d204ea939ca5eba7ef51be9e4ae56a35177413caabe849315086d3d9d2d31. No custom conflict resolution or live setting changed.The previous Linux qualification (212 focused tests and both real hash-locked installer controls) stays attributed to its tested source. The fresh native macOS attempt is runner-blocked: PyYAML is absent, and the Linux-oriented temporary-path fixture compares /tmp with macOS canonical /private/tmp. It is excluded from pass evidence. Fresh ordinary Ubuntu/hash-locked CI and exact combined-source review are required before relying on this head. Diff and source-blob checks pass.
Adoption still requires separately reviewed root/bootstrap and event policy, current caller compatibility, a checks-only App installed solely on this public pilot, protected Environment controls, live safe-success/deny/fork/wrong-issuer/stale/malformed/concurrent/ambiguous-write controls and an issuer-bound required rule. Current read-only configuration shows no pilot App, App variable or key; existing required rules are unchanged. New account/credential authorization, installation and activation are separate from source review. Related #237 and AI #27.