fix: Verify dispatcher provenance before first install#1791
Conversation
Keep legacy pin readers compatible with additive fields while making bootstrap-specific tag and manifest inputs fail closed when absent or malformed.
Keep the new additive-pin compatibility test compliant with the Go formatter.
Make runtime verification and release automation share one implementation and one fixed release identity policy instead of duplicating Sigstore checks.
Verify the immutable dispatcher Release with the shared Sigstore policy before recording its asset digests, so Unity bootstrap never trusts sibling checksums.
Reject malformed or stale bootstrap pins before Unity installation and verify published subjects whenever a security-relevant path changes.
Fail before extraction when no verified manifest is supplied, preventing a same-origin checksum from authenticating a first installation.
Run published subject verification whenever a pull request changes a trust input, or when no pull request diff exists. The guard now fails closed when attestation subjects and release assets are not the same complete set.
Use the package-stamped immutable dispatcher tag and manifest instead of a minimum-derived tag or release checksum sidecar. The installer command verifies the downloaded script digest before execution and propagates failures through the setup result contract.
Report installer source drift without blocking the release required to refresh the package pin, and document the immutable CLI bootstrap and package release order.
Clarify that digest pinning authenticates a release but cannot revoke an older published dispatcher, and record the required minimum-version remediation.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughDispatcher releases now use Sigstore verification, immutable release tags, and SHA-256 archive manifests. Unity installation consumes validated bootstrap pins, while release automation stamps and checks pins. Installer commands verify pinned digests, and documentation and CI enforce attested, failure-closed installation flows. ChangesDispatcher provenance and installation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant UnitySetup
participant PinReader
participant NativeInstaller
participant DispatcherRelease
participant AttestationVerifier
User->>UnitySetup: Start CLI installation
UnitySetup->>PinReader: Load dispatcher bootstrap pin
PinReader-->>UnitySetup: Release tag and archive manifest
UnitySetup->>NativeInstaller: Build installation command
NativeInstaller->>DispatcherRelease: Download installer script
NativeInstaller->>AttestationVerifier: Verify pinned script digest
AttestationVerifier-->>NativeInstaller: Verification result
NativeInstaller->>DispatcherRelease: Execute verified installer with manifest
DispatcherRelease-->>UnitySetup: Installation result
Possibly related PRs
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Provide the authenticated archive manifest required by the installer fixture so release selection tests continue to exercise their intended behavior.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Assets/Tests/Editor/CliSetupApplicationServiceTests.cs (1)
19-32: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDerive the expected dispatcher tag from the stamped package pin.
Both tests hard-code the current beta tag, so the next automated pin stamp will make otherwise-correct release changes fail CI. Load
DispatcherReleaseTagthroughLoadDispatcherBootstrapPin()as the expected value.Proposed update
- Is.EqualTo("dispatcher-v3.0.1-beta.6")); + Is.EqualTo(ExpectedDispatcherReleaseTag())); - Is.EqualTo("install dispatcher-v3.0.1-beta.6")); + Is.EqualTo($"install {ExpectedDispatcherReleaseTag()}")); +private static string ExpectedDispatcherReleaseTag() +{ + DispatcherBootstrapPinLoadResult result = + new CliPinReaderService().LoadDispatcherBootstrapPin(); + Assert.That(result.Success, Is.True, result.ErrorMessage); + return result.DispatcherReleaseTag; +}Also applies to: 48-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Assets/Tests/Editor/CliSetupApplicationServiceTests.cs` around lines 19 - 32, Update the tests around InstallGlobalCliAsync_UsesPinnedDispatcherReleaseTag and the corresponding test at lines 48–64 to derive the expected value from LoadDispatcherBootstrapPin().DispatcherReleaseTag instead of hard-coding "dispatcher-v3.0.1-beta.6", then compare InstalledVersion against that loaded pin value.
🧹 Nitpick comments (2)
cli/release-automation/cmd/check-dispatcher-pin/main_test.go (1)
5-15: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover every trust-policy input in the watched-path test.
Add
cli/dispatcher/attestation/policy.goandcli/dispatcher/attestation/trusted_root.json. Changes to either alter provenance verification and must trigger the network guard; testing onlyverifier.goleaves that contract unprotected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/release-automation/cmd/check-dispatcher-pin/main_test.go` around lines 5 - 15, Extend TestDispatcherPinPathNeedsNetworkVerificationRecognizesEveryWatchedPath to include cli/dispatcher/attestation/policy.go and cli/dispatcher/attestation/trusted_root.json in the watched paths, preserving coverage for all existing trust-policy inputs.Assets/Tests/Editor/CliPinReaderServiceTests.cs (1)
297-328: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTest CRLF and duplicate assets independently.
The input violates both rules, so either validation could regress while this test still passes. Use separate cases for non-LF line endings and duplicate asset names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Assets/Tests/Editor/CliPinReaderServiceTests.cs` around lines 297 - 328, Split LoadDispatcherBootstrapPinFromPath_WhenManifestUsesCrLfOrDuplicateAsset_ReturnsFailure into separate tests: one manifest containing only a CRLF line ending and one containing only duplicate asset names. Keep each test asserting result.Success is false so either validation rule is independently covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-test.yml:
- Around line 76-82: Update the “Verify published dispatcher pin subjects”
workflow step to pass github.base_ref through the step’s env block, then
reference that environment variable in the run command instead of interpolating
the GitHub expression directly. Preserve the existing origin/ branch argument
and HEAD value.
In `@cli/dispatcher/attestation/verifier.go`:
- Around line 222-233: Update isHexCommitSHA to accept uppercase hexadecimal
characters A through F in addition to digits and lowercase a through f, while
preserving the existing 40-character length requirement and rejecting all
non-hex characters.
In `@cli/release-automation/cmd/stamp-dispatcher-pin/main.go`:
- Around line 35-36: Update the command around StampDispatcherPin to generate
the verified stamp once, then apply it to both
Packages/src/project-runner-pin.json and .uloop/project-runner-pin.json. Ensure
either update failure returns an error, while preserving all existing fields and
making only additive pin changes.
In `@Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs`:
- Around line 121-129: Update LoadDispatcherBootstrapPinFromPath to catch
IOException and UnauthorizedAccessException thrown by LoadPinFromPath,
converting only those expected file-access failures into
DispatcherBootstrapPinLoadResult.FromFailure with the exception details. Leave
unexpected exceptions unhandled and preserve the existing pinResult failure and
validation paths.
---
Outside diff comments:
In `@Assets/Tests/Editor/CliSetupApplicationServiceTests.cs`:
- Around line 19-32: Update the tests around
InstallGlobalCliAsync_UsesPinnedDispatcherReleaseTag and the corresponding test
at lines 48–64 to derive the expected value from
LoadDispatcherBootstrapPin().DispatcherReleaseTag instead of hard-coding
"dispatcher-v3.0.1-beta.6", then compare InstalledVersion against that loaded
pin value.
---
Nitpick comments:
In `@Assets/Tests/Editor/CliPinReaderServiceTests.cs`:
- Around line 297-328: Split
LoadDispatcherBootstrapPinFromPath_WhenManifestUsesCrLfOrDuplicateAsset_ReturnsFailure
into separate tests: one manifest containing only a CRLF line ending and one
containing only duplicate asset names. Keep each test asserting result.Success
is false so either validation rule is independently covered.
In `@cli/release-automation/cmd/check-dispatcher-pin/main_test.go`:
- Around line 5-15: Extend
TestDispatcherPinPathNeedsNetworkVerificationRecognizesEveryWatchedPath to
include cli/dispatcher/attestation/policy.go and
cli/dispatcher/attestation/trusted_root.json in the watched paths, preserving
coverage for all existing trust-policy inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 538d510c-dc88-403d-82c2-d63966b1ea4b
⛔ Files ignored due to path filters (5)
cli/dispatcher/attestation/testdata/happy_asset_digest.txtis excluded by none and included by nonecli/dispatcher/attestation/testdata/happy_commit_sha.txtis excluded by none and included by nonecli/release-automation/go.modis excluded by none and included by nonecli/release-automation/go.sumis excluded by!**/*.sumand included by nonescripts/install.ps1is excluded by none and included by none
📒 Files selected for processing (45)
.github/workflows/build-and-test.yml.uloop/project-runner-pin.jsonAssets/Tests/Editor/CliPathSetupFlowTests.csAssets/Tests/Editor/CliPinReaderServiceTests.csAssets/Tests/Editor/CliSetupApplicationServiceTests.csAssets/Tests/Editor/NativeCliInstallerTests.csPackages/src/Editor/Application/CliPinReader.csPackages/src/Editor/Application/CliSetupApplicationService.csPackages/src/Editor/Domain/CliConstants.csPackages/src/Editor/Infrastructure/CLI/CliPinReaderService.csPackages/src/Editor/Infrastructure/CLI/NativeCliCommandBuilder.csPackages/src/Editor/Infrastructure/CLI/NativeCliInstaller.csPackages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.csPackages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.csPackages/src/project-runner-pin.jsonREADME.mdREADME_ja.mdSECURITY.mdcli/dispatcher/attestation/errors.gocli/dispatcher/attestation/fetcher.gocli/dispatcher/attestation/policy.gocli/dispatcher/attestation/policy_test.gocli/dispatcher/attestation/testdata/happy_bundle.jsoncli/dispatcher/attestation/trusted_root.gocli/dispatcher/attestation/trusted_root.jsoncli/dispatcher/attestation/trusted_root_test.gocli/dispatcher/attestation/verifier.gocli/dispatcher/attestation/verifier_test.gocli/dispatcher/cmd/refresh-attestation-trusted-root/main.gocli/dispatcher/internal/dispatcher/attestation_release.gocli/dispatcher/internal/dispatcher/attestation_verify.gocli/dispatcher/internal/dispatcher/attestation_verify_test.gocli/dispatcher/internal/dispatcher/dispatcher_test.gocli/dispatcher/shared-inputs-stamp.jsoncli/release-automation/cmd/check-dispatcher-pin/main.gocli/release-automation/cmd/check-dispatcher-pin/main_test.gocli/release-automation/cmd/stamp-dispatcher-pin/main.gocli/release-automation/internal/architecture/architecture_test.gocli/release-automation/internal/automation/dispatcher_pin_guard.gocli/release-automation/internal/automation/dispatcher_pin_guard_test.gocli/release-automation/internal/automation/dispatcher_pin_stamp.gocli/release-automation/internal/automation/dispatcher_pin_stamp_test.godocs/dispatcher-pin-release-order.mdscripts/install.shscripts/test-install-archive-manifest.sh
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Assets/Tests/Editor/CliSetupApplicationServiceTests.cs (1)
19-32: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDerive the expected dispatcher tag from the stamped package pin.
Both tests hard-code the current beta tag, so the next automated pin stamp will make otherwise-correct release changes fail CI. Load
DispatcherReleaseTagthroughLoadDispatcherBootstrapPin()as the expected value.Proposed update
- Is.EqualTo("dispatcher-v3.0.1-beta.6")); + Is.EqualTo(ExpectedDispatcherReleaseTag())); - Is.EqualTo("install dispatcher-v3.0.1-beta.6")); + Is.EqualTo($"install {ExpectedDispatcherReleaseTag()}")); +private static string ExpectedDispatcherReleaseTag() +{ + DispatcherBootstrapPinLoadResult result = + new CliPinReaderService().LoadDispatcherBootstrapPin(); + Assert.That(result.Success, Is.True, result.ErrorMessage); + return result.DispatcherReleaseTag; +}Also applies to: 48-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Assets/Tests/Editor/CliSetupApplicationServiceTests.cs` around lines 19 - 32, Update the tests around InstallGlobalCliAsync_UsesPinnedDispatcherReleaseTag and the corresponding test at lines 48–64 to derive the expected value from LoadDispatcherBootstrapPin().DispatcherReleaseTag instead of hard-coding "dispatcher-v3.0.1-beta.6", then compare InstalledVersion against that loaded pin value.
🧹 Nitpick comments (2)
cli/release-automation/cmd/check-dispatcher-pin/main_test.go (1)
5-15: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover every trust-policy input in the watched-path test.
Add
cli/dispatcher/attestation/policy.goandcli/dispatcher/attestation/trusted_root.json. Changes to either alter provenance verification and must trigger the network guard; testing onlyverifier.goleaves that contract unprotected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/release-automation/cmd/check-dispatcher-pin/main_test.go` around lines 5 - 15, Extend TestDispatcherPinPathNeedsNetworkVerificationRecognizesEveryWatchedPath to include cli/dispatcher/attestation/policy.go and cli/dispatcher/attestation/trusted_root.json in the watched paths, preserving coverage for all existing trust-policy inputs.Assets/Tests/Editor/CliPinReaderServiceTests.cs (1)
297-328: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTest CRLF and duplicate assets independently.
The input violates both rules, so either validation could regress while this test still passes. Use separate cases for non-LF line endings and duplicate asset names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Assets/Tests/Editor/CliPinReaderServiceTests.cs` around lines 297 - 328, Split LoadDispatcherBootstrapPinFromPath_WhenManifestUsesCrLfOrDuplicateAsset_ReturnsFailure into separate tests: one manifest containing only a CRLF line ending and one containing only duplicate asset names. Keep each test asserting result.Success is false so either validation rule is independently covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-and-test.yml:
- Around line 76-82: Update the “Verify published dispatcher pin subjects”
workflow step to pass github.base_ref through the step’s env block, then
reference that environment variable in the run command instead of interpolating
the GitHub expression directly. Preserve the existing origin/ branch argument
and HEAD value.
In `@cli/dispatcher/attestation/verifier.go`:
- Around line 222-233: Update isHexCommitSHA to accept uppercase hexadecimal
characters A through F in addition to digits and lowercase a through f, while
preserving the existing 40-character length requirement and rejecting all
non-hex characters.
In `@cli/release-automation/cmd/stamp-dispatcher-pin/main.go`:
- Around line 35-36: Update the command around StampDispatcherPin to generate
the verified stamp once, then apply it to both
Packages/src/project-runner-pin.json and .uloop/project-runner-pin.json. Ensure
either update failure returns an error, while preserving all existing fields and
making only additive pin changes.
In `@Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs`:
- Around line 121-129: Update LoadDispatcherBootstrapPinFromPath to catch
IOException and UnauthorizedAccessException thrown by LoadPinFromPath,
converting only those expected file-access failures into
DispatcherBootstrapPinLoadResult.FromFailure with the exception details. Leave
unexpected exceptions unhandled and preserve the existing pinResult failure and
validation paths.
---
Outside diff comments:
In `@Assets/Tests/Editor/CliSetupApplicationServiceTests.cs`:
- Around line 19-32: Update the tests around
InstallGlobalCliAsync_UsesPinnedDispatcherReleaseTag and the corresponding test
at lines 48–64 to derive the expected value from
LoadDispatcherBootstrapPin().DispatcherReleaseTag instead of hard-coding
"dispatcher-v3.0.1-beta.6", then compare InstalledVersion against that loaded
pin value.
---
Nitpick comments:
In `@Assets/Tests/Editor/CliPinReaderServiceTests.cs`:
- Around line 297-328: Split
LoadDispatcherBootstrapPinFromPath_WhenManifestUsesCrLfOrDuplicateAsset_ReturnsFailure
into separate tests: one manifest containing only a CRLF line ending and one
containing only duplicate asset names. Keep each test asserting result.Success
is false so either validation rule is independently covered.
In `@cli/release-automation/cmd/check-dispatcher-pin/main_test.go`:
- Around line 5-15: Extend
TestDispatcherPinPathNeedsNetworkVerificationRecognizesEveryWatchedPath to
include cli/dispatcher/attestation/policy.go and
cli/dispatcher/attestation/trusted_root.json in the watched paths, preserving
coverage for all existing trust-policy inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 538d510c-dc88-403d-82c2-d63966b1ea4b
⛔ Files ignored due to path filters (5)
cli/dispatcher/attestation/testdata/happy_asset_digest.txtis excluded by none and included by nonecli/dispatcher/attestation/testdata/happy_commit_sha.txtis excluded by none and included by nonecli/release-automation/go.modis excluded by none and included by nonecli/release-automation/go.sumis excluded by!**/*.sumand included by nonescripts/install.ps1is excluded by none and included by none
📒 Files selected for processing (45)
.github/workflows/build-and-test.yml.uloop/project-runner-pin.jsonAssets/Tests/Editor/CliPathSetupFlowTests.csAssets/Tests/Editor/CliPinReaderServiceTests.csAssets/Tests/Editor/CliSetupApplicationServiceTests.csAssets/Tests/Editor/NativeCliInstallerTests.csPackages/src/Editor/Application/CliPinReader.csPackages/src/Editor/Application/CliSetupApplicationService.csPackages/src/Editor/Domain/CliConstants.csPackages/src/Editor/Infrastructure/CLI/CliPinReaderService.csPackages/src/Editor/Infrastructure/CLI/NativeCliCommandBuilder.csPackages/src/Editor/Infrastructure/CLI/NativeCliInstaller.csPackages/src/Editor/Presentation/Setup/SetupWizardCliWorkflowController.csPackages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.csPackages/src/project-runner-pin.jsonREADME.mdREADME_ja.mdSECURITY.mdcli/dispatcher/attestation/errors.gocli/dispatcher/attestation/fetcher.gocli/dispatcher/attestation/policy.gocli/dispatcher/attestation/policy_test.gocli/dispatcher/attestation/testdata/happy_bundle.jsoncli/dispatcher/attestation/trusted_root.gocli/dispatcher/attestation/trusted_root.jsoncli/dispatcher/attestation/trusted_root_test.gocli/dispatcher/attestation/verifier.gocli/dispatcher/attestation/verifier_test.gocli/dispatcher/cmd/refresh-attestation-trusted-root/main.gocli/dispatcher/internal/dispatcher/attestation_release.gocli/dispatcher/internal/dispatcher/attestation_verify.gocli/dispatcher/internal/dispatcher/attestation_verify_test.gocli/dispatcher/internal/dispatcher/dispatcher_test.gocli/dispatcher/shared-inputs-stamp.jsoncli/release-automation/cmd/check-dispatcher-pin/main.gocli/release-automation/cmd/check-dispatcher-pin/main_test.gocli/release-automation/cmd/stamp-dispatcher-pin/main.gocli/release-automation/internal/architecture/architecture_test.gocli/release-automation/internal/automation/dispatcher_pin_guard.gocli/release-automation/internal/automation/dispatcher_pin_guard_test.gocli/release-automation/internal/automation/dispatcher_pin_stamp.gocli/release-automation/internal/automation/dispatcher_pin_stamp_test.godocs/dispatcher-pin-release-order.mdscripts/install.shscripts/test-install-archive-manifest.sh
🛑 Comments failed to post (4)
.github/workflows/build-and-test.yml (1)
76-82: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent script injection by passing GitHub context via environment variables.
Interpolating GitHub context expressions like
${{ github.base_ref }}directly into a bash script can lead to command injection if a branch name contains malicious characters (such as double quotes or backticks). It is safer to bind the context to an environment variable and reference that variable within the script.🔒️ Proposed fix
- name: Verify published dispatcher pin subjects if: github.event_name == 'pull_request' working-directory: cli/release-automation env: GITHUB_TOKEN: ${{ github.token }} + BASE_REF: ${{ github.base_ref }} - run: go run ./cmd/check-dispatcher-pin --base "origin/${{ github.base_ref }}" --head HEAD + run: go run ./cmd/check-dispatcher-pin --base "origin/$BASE_REF" --head HEAD📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- name: Verify published dispatcher pin subjects if: github.event_name == 'pull_request' working-directory: cli/release-automation env: GITHUB_TOKEN: ${{ github.token }} BASE_REF: ${{ github.base_ref }} run: go run ./cmd/check-dispatcher-pin --base "origin/$BASE_REF" --head HEAD🧰 Tools
🪛 zizmor (1.26.1)
[error] 81-81: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-and-test.yml around lines 76 - 82, Update the “Verify published dispatcher pin subjects” workflow step to pass github.base_ref through the step’s env block, then reference that environment variable in the run command instead of interpolating the GitHub expression directly. Preserve the existing origin/ branch argument and HEAD value.cli/dispatcher/attestation/verifier.go (1)
222-233: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Support uppercase characters in hex validation.
The current validation logic strictly requires lowercase hex characters, which causes valid uppercase hex hashes to be rejected. Expanding the check to allow
'A'through'F'aligns it with standard hex-parsing behavior (likehex.DecodeString, which is used elsewhere in this file) and prevents false negatives if uppercase SHAs are passed.💡 Proposed fix
func isHexCommitSHA(s string) bool { if len(s) != 40 { return false } for i := 0; i < len(s); i++ { c := s[i] - if (c < '0' || c > '9') && (c < 'a' || c > 'f') { + if (c < '0' || c > '9') && (c < 'a' || c > 'f') && (c < 'A' || c > 'F') { return false } } return true }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func isHexCommitSHA(s string) bool { if len(s) != 40 { return false } for i := 0; i < len(s); i++ { c := s[i] if (c < '0' || c > '9') && (c < 'a' || c > 'f') && (c < 'A' || c > 'F') { return false } } return true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/dispatcher/attestation/verifier.go` around lines 222 - 233, Update isHexCommitSHA to accept uppercase hexadecimal characters A through F in addition to digits and lowercase a through f, while preserving the existing 40-character length requirement and rejecting all non-hex characters.cli/release-automation/cmd/stamp-dispatcher-pin/main.go (1)
35-36: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Stamp the mirrored
.ulooppin as well.This command updates only
Packages/src/project-runner-pin.json, leaving.uloop/project-runner-pin.jsonstale. Generate the verified stamp once and write it to both mirrors, failing if either update fails.As per path instructions, “The project runner pin (
Packages/src/project-runner-pin.json, mirrored to.uloop/project-runner-pin.json) must preserve its existing fields and evolve additively only.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/release-automation/cmd/stamp-dispatcher-pin/main.go` around lines 35 - 36, Update the command around StampDispatcherPin to generate the verified stamp once, then apply it to both Packages/src/project-runner-pin.json and .uloop/project-runner-pin.json. Ensure either update failure returns an error, while preserving all existing fields and making only additive pin changes.Source: Path instructions
Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs (1)
121-129: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return a structured failure for expected pin-file I/O errors.
LoadPinFromPathcan throwIOExceptionorUnauthorizedAccessExceptionduringFile.ReadAllText, causing installation to escape the new failure-result contract. Convert only these expected file-access failures intoFromFailure.Based on learnings, catch only expected domain-specific exceptions and let unexpected defects surface.
Proposed handling
- string content = File.ReadAllText(pinPath); + string content; + try + { + content = File.ReadAllText(pinPath); + } + catch (IOException ex) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file at {pinPath} could not be read: {ex.Message}"); + } + catch (UnauthorizedAccessException ex) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file at {pinPath} could not be read: {ex.Message}"); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs` around lines 121 - 129, Update LoadDispatcherBootstrapPinFromPath to catch IOException and UnauthorizedAccessException thrown by LoadPinFromPath, converting only those expected file-access failures into DispatcherBootstrapPinLoadResult.FromFailure with the exception details. Leave unexpected exceptions unhandled and preserve the existing pinResult failure and validation paths.Source: Learnings
Derive the archive manifest from the verified release bundle before executing the fail-closed installer, and cover that README bootstrap transformation.
7ee46c7
into
security/cli-distribution-hardening-integration
Summary
Root cause
Unity derived a dispatcher tag from its minimum version and trusted a same-origin checksum sidecar for an installer script. That checksum is integrity-only and did not authenticate release provenance.
Changes
Attack conditions addressed
An attacker must no longer be able to substitute a mutable branch installer, a same-origin checksum, an unmanifested script, or an archive whose digest is absent from the verified pin. Network, bundle, identity, tag, and digest failures fail closed.
Verification
Residual risk