Harden Aspire skills bundle loading, validation, and caching - #19068
Ella Hathaway (ellahathaway) merged 21 commits into
Conversation
Track archive digests in the version cache, replace stale same-version content, preserve verified offline fallback, and separate bundle construction from installer acquisition and cache policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b41df8ad-bb69-4f8c-9447-88f3fd5206d3
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
PR testingResult: ✅ Local source verification passed All validation below ran locally on Windows against exact PR head Automated validation
Local end-to-end validationThese were process-level scenarios, not unit tests. I rebuilt
Each scenario used a fresh temporary workspace and an isolated
The published
The GitHub scenarios emulated CLI No PR-specific failures found. Official packaged-artifact and hosted-CI validation remain pending because the GitHub jobs were still queued. |
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Focused source testsObjective: Exercise bundle validation, digest invalidation, provenance, cache locking/cleanup, embedded fallback, and Agent Init integration at the exact PR head. Coverage Type: Automated unit/integration Status: PASS Command: dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class '*.AspireSkillsInstallerTests' --filter-class '*.AspireSkillsBundleTests' --filter-class '*.AgentInitCommandTests' --filter-not-trait 'quarantined=true' --filter-not-trait 'outerloop=true'Result: 88 passed, 0 failed, 0 skipped. Evidence: Scenario 2: Embedded bundle installation and matching-cache reuseObjective: Run the real source-built CLI against fresh workspaces with remote fetch disabled, install the Coverage Type: Happy path Status: PASS Result: Evidence:
Scenario 3: Embedded same-version digest mismatchObjective: Simulate stale content for the same bundle version by replacing the digest marker and adding a stale sentinel. Coverage Type: Unhappy path Status: PASS Expected Outcome: Reject and replace the stale cache, remove stale content, restore the trusted embedded digest, and install the requested skill. Result: The cache was replaced atomically, the sentinel was removed, and digest Evidence:
Scenario 4: Verified GitHub acquisition and cache reuseObjective: Exercise the opt-in remote-fetch path against the real Coverage Type: Happy path / provenance Status: PASS Identity: Result: The release was downloaded and attested, Evidence:
Scenario 5: GitHub same-version digest mismatchObjective: Change the digest of a provenance-marked cache and confirm the current GitHub release replaces it. Coverage Type: Unhappy path Status: PASS Expected Outcome: Download and re-attest the release, replace stale content, and restore both digest and provenance markers. Result: The stale sentinel was removed, the release digest was restored, and the GitHub attestation marker was recreated. Evidence:
Scenario 6: Verified GitHub cache while offlineObjective: Force GitHub requests through a refused local proxy and confirm a previously verified GitHub cache remains usable. Coverage Type: Boundary / recovery Status: PASS Expected Outcome: Reuse only the provenance-marked cache when release metadata cannot be fetched. Result: Debug output recorded GitHub acquisition failure followed by reuse of the previously verified cache. Its sentinel, digest, and attestation marker remained intact. Evidence:
Scenario 7: Untrusted mismatched cache while offlineObjective: Remove the GitHub attestation marker, change the digest, block GitHub, and verify the cache cannot assert its own provenance or identity. Coverage Type: Security boundary / unhappy path Status: PASS Expected Outcome: Reject the untrusted cache and replace it from the trusted embedded bundle without creating a GitHub attestation marker. Result: Debug output explicitly rejected the missing provenance and mismatched digest. The stale sentinel was removed, the embedded digest was restored, and no GitHub attestation marker was present afterward. Evidence:
Scenario Not ExecutedOfficial dogfood CLI installation and template smoke testStatus: BLOCKED No dogfood comment or installable packaged artifact was available because the workflow remained queued. Per the PR-testing artifact gate, the official CLI version could not be matched to the PR head, so artifact-based Summary
Overall ResultSOURCE BEHAVIOR VERIFIED; PACKAGED PR ARTIFACT PENDING No PR-specific failures were found in the exact-head source tests or process-level scenarios. Full PR verification remains incomplete until the official dogfood artifact is published and its reported commit matches the PR head. RecommendationOnce workflow run 31125094622 publishes the dogfood comment, run only the remaining artifact-version check and a minimal packaged |
David Pine (IEvangelist)
left a comment
David Pine (IEvangelist)
left a comment
There was a problem hiding this comment.
Found two correctness issues and one regression-coverage gap.
aa01db8 to
ed8f8c3
Compare
Preserve known GitHub digests during offline fallback, reject null manifest entries, and strengthen digest invalidation and CLI cache reuse coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b41df8ad-bb69-4f8c-9447-88f3fd5206d3
|
Copilot review |
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19068Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19068" |
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Fall back to verified cache or the embedded bundle for non-caller HTTP cancellation while preserving explicit caller cancellation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a679117e-daef-41d5-b8ce-013012e63e78
Track whether current GitHub release metadata was available so an advertised asset without a usable digest cannot revive an unpinned same-version cache leaf. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a679117e-daef-41d5-b8ce-013012e63e78
This comment has been minimized.
This comment has been minimized.
Exercise cancellation only after lock contention, prove cache reuse when the last-used marker cannot be updated, and remove redundant disabled-fetch cache coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a679117e-daef-41d5-b8ce-013012e63e78
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 2 / 101 test projects · 5 jobs, from 12 changed files. Selected test projects (2 / 101)
Selected jobs (5)
How these were chosen — grouped by what changed📦 affected project 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: Focused exact-head source suiteObjective: Exercise bundle/provider validation, SHA-512 and legacy SHA-256 handling, GitHub mapping, metadata availability, timeouts, cancellation, cache locking/cleanup, embedded fallback, and Agent Init integration. Coverage Type: Automated unit/integration Status: Passed Result: 127 passed, 0 failed, 0 skipped. Evidence: Scenario 2: Fresh packaged embedded installationObjective: Run the official PR CLI with remote fetch left at its production default. Coverage Type: Happy path Status: Passed Result: Debug output reported remote fetch disabled. The CLI installed Evidence:
Scenario 3: Exact SHA-512 cache reuseObjective: Prove a matching embedded leaf is loaded rather than replaced. Coverage Type: Happy path / cache hit Status: Passed Result: Debug output reported a cache hit and a sentinel placed in the leaf survived the second Agent Init run. Evidence: Scenario 4: Corrupt SHA-512 marker recoveryObjective: Corrupt Coverage Type: Unhappy path / recovery Status: Passed Expected Outcome: Reject the invalid leaf, restore the trusted SHA-512 identity, remove stale content, and reinstall all requested skills. Result: The marker was restored to the 128-character leaf identity, the sentinel was removed, and all four skills were reinstalled. Evidence: Scenario 5: Real GitHub acquisition, attestation, and mappingObjective: Enable the hidden remote-fetch feature explicitly and acquire the real Coverage Type: Happy path / provenance Status: Passed Result: The CLI honored explicit configuration, verified GitHub provenance, created Evidence:
Scenario 6: Offline verified-cache fallbackObjective: Block GitHub through a refused local proxy while a provenance-marked leaf exists. Coverage Type: Boundary / recovery Status: Passed Expected Outcome: Reuse only the verified GitHub leaf when release metadata cannot be fetched. Result: Debug output reported the verified offline fallback, and the cache sentinel and attestation marker survived. Evidence: Scenario 7: Offline untrusted-cache rejectionObjective: Remove provenance, corrupt the archive marker, block GitHub, and rerun Agent Init. Coverage Type: Security boundary / unhappy path Status: Passed Expected Outcome: Reject the untrusted cache and replace it from the embedded snapshot without retaining GitHub provenance metadata. Result: The stale sentinel was removed, the SHA-512 marker was restored, all requested skills were installed, and both Evidence:
Scenario 8: PR hive template and AppHost lifecycle smokeObjective: Create a fresh Coverage Type: Packaged CLI smoke Status: Passed Result: Evidence:
Summary
Overall ResultPR VERIFIED The official PR artifact matches the latest head and all approved happy-path, recovery, provenance, offline, and packaged CLI scenarios passed. No PR-specific failures were found. |
Adam Ratzman (adamint)
left a comment
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
Looks good overall; one cache-migration cleanup gap remains.
|
✅ No documentation update needed. Step 5 branch taken: Triggered signals: none (signal_count: 0, recommendation: docs_optional) Rationale: All 12 changed files are internal CLI implementation/test files under |
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
Description
The Aspire skills cache was keyed only by bundle version. If the contents of a published archive changed without a version change, the CLI continued loading the old extracted bundle from disk.
This PR makes the archive's SHA-512 digest part of the cache identity by storing each extracted bundle under
<bundle-version>/<archive-sha512>. A CLI with a known embedded SHA-512 reuses only that exact cache leaf. Multiple same-version generations can coexist so CLIs carrying different snapshots do not replace each other's cache; inactive leaves age out according to their.lastusedtimestamps. Legacy flat cache entries are removed when a digest-addressed leaf is published. There is no migration for the unshipped.archive-sha256layout.GitHub release metadata and artifact attestations still identify release assets by SHA-256. GitHub-sourced cache leaves therefore persist
.github-archive-sha256alongside.archive-sha512; the installer uses that mapping to find the matching SHA-512 leaf before downloading the asset again. Downloaded bytes must match the advertised GitHub SHA-256 when one is present, while SHA-512 remains the bundle/cache integrity identity. The attestation verifier remains SHA-256-based.The acquisition flow was also reorganized so each type has a clearer responsibility:
AspireSkillsInstallerselects the source and owns download, attestation, caching, locking, fallback, and cleanup policy.AspireSkillsBundleProviderverifies and extracts archives, validates manifests and files, and creates or loads bundles.EmbeddedAspireSkillsBundleProvideradapts the bundle embedded in the CLI and delegates bundle creation to the shared provider.AspireSkillsBundleis a validated in-memory model, allowing its source directory to be replaced or removed safely after loading.Remote fetching remains hidden and disabled by default, while explicit configuration is still honored. Network failures, truncated responses, and timeouts fall back to a previously verified matching cache or the embedded snapshot without swallowing caller cancellation.
Additional validation rejects unsafe archive paths and skill names, missing or excluded
SKILL.mdfiles, duplicate paths, incompatible manifests, archive digest mismatches, and files whose hashes do not match the manifest. Current bundles use per-file SHA-512; the already-attested v0.0.1 archive retains its legacy per-file SHA-256 hashes.Offline behavior remains provenance-aware: when GitHub is unavailable, the CLI may reuse a cached GitHub bundle only when it was previously verified against the expected workflow. Installer-owned digest, provenance, and freshness markers are removed from extracted archive content and recreated from the acquisition result, so an archive cannot assert its own cache identity or provenance.
Fixes #19025