Add embedded Aspire skills fallback#17537
Conversation
IEvangelist
added
area-engineering-systems
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17537Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17537" |
Embed a checked-in Aspire skills bundle snapshot for CLI fallback, make agent init warn instead of fail when bundle acquisition is unavailable, and add automation to refresh the snapshot via a draft PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
IEvangelist
force-pushed
the
dapine/skills-bundle-fallback
branch
from
8e7cd7d to
8f189b0
Compare
IEvangelist
requested review from
JamesNK,
davidfowl and
mitchdenny
as code owners
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Safeguard suggestion: add CI verification of the embedded .tgz against its Sigstore attestation.
Today the runtime check is "SHA in the .tgz matches SHA in aspire-skills.metadata.json" — both committed side-by-side. The Sigstore/gh attestation verify step only runs inside update-aspire-skills-bundle.yml, which an attacker landing a hand-crafted PR would simply not invoke. GitHub's PR UI renders binary diffs as Bin XXX → YYY bytes, so an atomic swap of the archive + metadata SHA is easy to miss in review.
Suggest adding a CI job (triggered on any PR that touches src/Aspire.Cli/Agents/AspireSkills/Embedded/**) that re-asserts the embedded archive really is a Sigstore-attested release artifact from microsoft/aspire-skills:
- name: Verify embedded Aspire skills bundle attestation
shell: pwsh
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
$meta = Get-Content -Raw src/Aspire.Cli/Agents/AspireSkills/Embedded/aspire-skills.metadata.json | ConvertFrom-Json
if ($meta.repository -ne 'microsoft/aspire-skills') {
throw "Unexpected embedded bundle repository '$($meta.repository)'."
}
$archive = "src/Aspire.Cli/Agents/AspireSkills/Embedded/$($meta.assetName)"
$actual = (Get-FileHash -Algorithm SHA256 $archive).Hash.ToLowerInvariant()
if ($actual -ne $meta.sha256) {
throw "Embedded bundle SHA-256 mismatch. Expected '$($meta.sha256)', got '$actual'."
}
$certIdentity = "https://github.com/$($meta.repository)/.github/workflows/publish.yml@refs/tags/$($meta.tag)"
gh attestation verify $archive `
--repo $meta.repository `
--cert-identity $certIdentity `
--cert-oidc-issuer 'https://token.actions.githubusercontent.com'This closes the gap without sacrificing offline builds — the committed .tgz stays a build-time convenience, and CI enforces that any change to it (whether by the bot workflow or a hand-crafted PR) must correspond to a Sigstore-attested release. The repository pin guards against an attacker swapping the metadata to point at a repo they control.
mitchdenny
left a comment
mitchdenny
left a comment
There was a problem hiding this comment.
Additional findings from the review (one bug-class behavioral regression, one dead code path, one workflow-input injection vector, and two diagnostic/UX improvements).
Avoid PowerShell interpolation of workflow inputs, simplify failure handling, improve embedded archive hash diagnostics, and keep embedded metadata validation messages consistently English. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@IEvangelist Thanks for the quick turnaround on the fixes — dead-code, workflow-injection, SHA-256 diagnostic, and metadata-error consistency all look good. One outstanding item from the original review I want to confirm is intentional before this auto-merges: the removal of The rationale "we always have an embedded fallback" holds for the prompt-driven path, but the embedded fallback can itself fail (corrupted/missing embedded resource, SHA mismatch, metadata mismatch — all paths added by this PR return Is the loss of the non-zero exit code on the explicit path intentional? If yes, no objection — just worth calling out in the PR description so downstream automation owners know. If not, keeping Strict for |
Yes, this is intentional. The goal is for |
Add a PR workflow and reusable script that verify the checked-in Aspire skills bundle archive hash and GitHub artifact attestation whenever the embedded snapshot changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Added this safeguard. There is now a dedicated |
|
End-to-end tested in the repo container runner (PR CLI
|
|
/backport to release/13.4 |
|
Started backporting to |
|
@IEvangelist backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Add embedded Aspire skills fallback
Using index info to reconstruct a base tree...
M src/Aspire.Cli/Commands/AgentInitCommand.cs
M src/Aspire.Cli/Resources/AgentCommandStrings.Designer.cs
M src/Aspire.Cli/Resources/AgentCommandStrings.resx
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.cs.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.de.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.es.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.fr.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.it.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.ja.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.ko.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.pl.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.pt-BR.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.ru.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.tr.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.zh-Hans.xlf
M src/Aspire.Cli/Resources/xlf/AgentCommandStrings.zh-Hant.xlf
M tests/Aspire.Cli.Tests/Commands/AgentInitCommandTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Aspire.Cli/Commands/AgentInitCommand.cs
CONFLICT (content): Merge conflict in src/Aspire.Cli/Commands/AgentInitCommand.cs
Auto-merging src/Aspire.Cli/Resources/AgentCommandStrings.Designer.cs
Auto-merging src/Aspire.Cli/Resources/AgentCommandStrings.resx
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.cs.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.de.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.es.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.fr.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.it.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.ja.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.ko.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.pl.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.pt-BR.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.ru.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.tr.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.zh-Hans.xlf
Auto-merging src/Aspire.Cli/Resources/xlf/AgentCommandStrings.zh-Hant.xlf
Auto-merging tests/Aspire.Cli.Tests/Commands/AgentInitCommandTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Add embedded Aspire skills fallback
Error: The process '/usr/bin/git' failed with exit code 128 |
| { | ||
| if (string.IsNullOrWhiteSpace(metadata.Version)) | ||
| { | ||
| return "Embedded Aspire skills metadata must specify a version."; |
There was a problem hiding this comment.
resource strings for localization please
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26518122569 |
… init Document the best-effort skills bundle acquisition order (cache → GitHub release asset → embedded snapshot) introduced in microsoft/aspire#17537. The command now warns and skips bundle-backed skills instead of failing when all acquisition sources are unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1094
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1094 targeting Updated Note This draft PR needs human review before merging. |
* Add embedded Aspire skills fallback (#17537) Embed a checked-in Aspire skills bundle snapshot for CLI fallback, make agent init warn instead of fail when bundle acquisition is unavailable, and add automation to refresh the snapshot via a draft PR. Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix release backport resource strings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix Docker compose prepare test runtime dependency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Make Aspire skills installation reliable when GitHub release asset acquisition is unavailable.
Users running
aspire agent initdirectly, or through the chainedaspire new|initflow, now get a non-fatal warning if the Aspire skills bundle cannot be resolved instead of having the whole command fail. The installer now resolves the skills bundle in this order:microsoft/aspire-skillsGitHub release asset.This also adds a scheduled/manual automation workflow that downloads the configured
microsoft/aspire-skillsrelease asset, verifies its GitHub artifact attestation, updates the checked-in embedded snapshot and metadata, and opens a draft PR with the Aspire bot. The sharedcreate-pull-requestaction now supports adraftinput for that workflow.User-facing usage
No new CLI options are required. Existing commands keep working and automatically use the embedded fallback when cache/GitHub acquisition is unavailable:
Behavior change
aspire agent initnow uses best-effort behavior for Aspire skills bundle acquisition in both explicit invocations and the chainedaspire new|initprompt. If every bundle source is unavailable or invalid, the command warns and skips only bundle-backed Aspire skills instead of returning a non-zero exit code solely because optional skills content could not be acquired. Other selected agent-environment configuration can still complete.Security considerations
This change touches release asset download, archive extraction, and a checked-in supply-chain fallback. GitHub downloads still require artifact attestation verification before use. The embedded snapshot is checked against embedded SHA-256 metadata before extraction, and the existing safe archive extraction and bundle manifest hash validation are reused for cache, GitHub, and embedded sources. The refresh workflow uses the existing Aspire bot GitHub App secrets and verifies the release asset attestation before writing the embedded snapshot.
Validation:
.\restore.cmd.\eng\scripts\update-aspire-skills-bundle.ps1.\eng\scripts\verify-aspire-skills-bundle.ps1gh attestation verify src\Aspire.Cli\Agents\AspireSkills\Embedded\aspire-skills-v0.0.1.tgz --repo microsoft/aspire-skills --cert-identity "https://github.com/microsoft/aspire-skills/.github/workflows/publish.yml@refs/tags/v0.0.1" --cert-oidc-issuer "https://token.actions.githubusercontent.com"dotnet build /t:UpdateXlf src\Aspire.Cli\Aspire.Cli.csprojdotnet 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"dotnet build src\Aspire.Cli\Aspire.Cli.csproj /p:SkipNativeBuild=truegit --no-pager diff --checkFixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?