Skip to content

Use Sigstore attestation helper APIs for provenance checking#14931

Merged
mitchdenny merged 4 commits intorelease/13.2from
sigstore-attestation-apis
Mar 5, 2026
Merged

Use Sigstore attestation helper APIs for provenance checking#14931
mitchdenny merged 4 commits intorelease/13.2from
sigstore-attestation-apis

Conversation

@mitchdenny
Copy link
Member

Description

Refactor SigstoreNpmProvenanceChecker to use the attestation helper APIs already available in Sigstore 0.3.0, replacing manual JSON parsing with typed APIs. Also removes the unused non-Sigstore NpmProvenanceChecker codepath.

What changed

Simplified provenance checking (~540 lines removed):

  • Removed ParseAttestation (~80 lines), ParseProvenanceFromStatement (~20 lines), and NpmAttestationParseResult class — replaced by ExtractSlsaBundleJson (simple bundle extraction) and ExtractProvenanceFromResult which uses VerificationResult.Statement (auto-populated InTotoStatement) and VerifiedIdentity.Extensions (parsed FulcioCertificateExtensions)
  • Uses bundle.DsseEnvelope.Payload directly instead of manual base64 decoding from JSON
  • Prefers certificate extensions (cryptographically bound to the signing certificate) over SLSA predicate JSON values for source repository and ref
  • Fixed JsonArray safety: uses pattern matching (is not JsonArray) instead of AsArray() to gracefully handle wrong JSON types

Removed dead code:

  • Deleted NpmProvenanceChecker.cs and NpmProvenanceCheckerTests.cs — production DI only registered SigstoreNpmProvenanceChecker, so this was unused code

Added comprehensive adversarial/security tests (68 total):

  • Malformed JSON attacks (deeply nested, truncated, wrong types, null/empty)
  • Attestation structure manipulation (swapped predicate types, mixed arrays, missing fields)
  • Provenance spoofing (homoglyph URLs, path traversal, null bytes in workflow paths)
  • Build type spoofing (query param injection, wrong domains, empty/null values)
  • Workflow ref manipulation (branch vs tag, path traversal in refs, empty names)
  • URL parsing edge cases (subdomain attacks, non-absolute URIs, extra path segments)
  • Statement extraction edge cases (missing predicate fields, empty predicates, no identity)

Security improvement

Source repository is now verified at two levels:

  1. Cryptographically via Fulcio certificate extensions (signed by the CA during OIDC token exchange)
  2. Defense-in-depth via SLSA predicate field comparison (retained from before)

Fixes # (issue)

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
    • No

Mitch Denny and others added 2 commits March 4, 2026 14:43
Replace manual JSON parsing in SigstoreNpmProvenanceChecker with the
new attestation APIs already available in Sigstore 0.3.0:

- Remove ParseAttestation (~80 lines) and ParseProvenanceFromStatement
  (~20 lines) methods and NpmAttestationParseResult class
- Add ExtractSlsaBundleJson to extract bundle JSON from npm response
- Add ExtractProvenanceFromResult using VerificationResult.Statement
  (InTotoStatement) and VerifiedIdentity.Extensions (FulcioCertificateExtensions)
- Use bundle.DsseEnvelope.Payload directly instead of manual base64 decode
- Prefer certificate extensions (cryptographically bound) over SLSA
  predicate values for source repository and ref
- Fix JsonArray safety: use pattern matching instead of AsArray()

Add comprehensive adversarial/security tests (68 total):
- Malformed JSON attacks (deeply nested, truncated, wrong types, null)
- Attestation structure manipulation (swapped predicates, mixed arrays)
- Provenance spoofing (homoglyph URLs, path traversal, null bytes)
- Build type spoofing (query params, wrong domains, empty values)
- URL parsing edge cases (subdomain attacks, non-absolute URIs)
- WorkflowRefInfo parsing edge cases (path traversal in names)
- Statement extraction edge cases (missing fields, empty predicates)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Delete NpmProvenanceChecker and its tests — production code only uses
SigstoreNpmProvenanceChecker (registered in DI), so the non-Sigstore
fallback was dead code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 4, 2026 03:46
@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14931

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14931"

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors npm provenance verification in the Aspire CLI to rely on Sigstore’s attestation helper/typed APIs (Sigstore 0.3.0) rather than manual JSON parsing, and removes the unused non-Sigstore provenance checker path.

Changes:

  • Simplified attestation handling by extracting the SLSA bundle JSON and using VerificationResult.Statement + VerifiedIdentity.Extensions to derive provenance fields.
  • Updated Sigstore verification flow to pass DSSE payload bytes directly (and return both failure + verification result from the verifier helper).
  • Removed the unused NpmProvenanceChecker implementation and its tests; expanded/adapted Sigstore provenance tests (including adversarial cases).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/Aspire.Cli/Npm/SigstoreNpmProvenanceChecker.cs Reworks parsing/verification to use Sigstore typed APIs and extracts provenance from verification results.
src/Aspire.Cli/Npm/NpmProvenanceChecker.cs Deletes unused legacy provenance checker implementation.
tests/Aspire.Cli.Tests/Agents/SigstoreNpmProvenanceCheckerTests.cs Updates tests for the new helper methods and adds extensive adversarial coverage.
tests/Aspire.Cli.Tests/Agents/NpmProvenanceCheckerTests.cs Deletes tests for the removed legacy checker.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

🎬 CLI E2E Test Recordings

The following terminal recordings are available for commit e26e75e:

Test Recording
AddPackageInteractiveWhileAppHostRunningDetached ▶️ View Recording
AddPackageWhileAppHostRunningDetached ▶️ View Recording
AgentCommands_AllHelpOutputs_AreCorrect ▶️ View Recording
AgentInitCommand_MigratesDeprecatedConfig ▶️ View Recording
AgentInitCommand_WithMalformedMcpJson_ShowsErrorAndExitsNonZero ▶️ View Recording
AspireUpdateRemovesAppHostPackageVersionFromDirectoryPackagesProps ▶️ View Recording
Banner_DisplayedOnFirstRun ▶️ View Recording
Banner_DisplayedWithExplicitFlag ▶️ View Recording
CreateAndDeployToDockerCompose ▶️ View Recording
CreateAndDeployToDockerComposeInteractive ▶️ View Recording
CreateAndPublishToKubernetes ▶️ View Recording
CreateAndRunAspireStarterProject ▶️ View Recording
CreateAndRunAspireStarterProjectWithBundle ▶️ View Recording
CreateAndRunJsReactProject ▶️ View Recording
CreateAndRunPythonReactProject ▶️ View Recording
CreateAndRunTypeScriptStarterProject ▶️ View Recording
CreateEmptyAppHostProject ▶️ View Recording
CreateStartAndStopAspireProject ▶️ View Recording
CreateStartWaitAndStopAspireProject ▶️ View Recording
CreateTypeScriptAppHostWithViteApp ▶️ View Recording
DescribeCommandResolvesReplicaNames ▶️ View Recording
DescribeCommandShowsRunningResources ▶️ View Recording
DetachFormatJsonProducesValidJson ▶️ View Recording
DoctorCommand_DetectsDeprecatedAgentConfig ▶️ View Recording
DoctorCommand_WithSslCertDir_ShowsTrusted ▶️ View Recording
DoctorCommand_WithoutSslCertDir_ShowsPartiallyTrusted ▶️ View Recording
LogsCommandShowsResourceLogs ▶️ View Recording
PsCommandListsRunningAppHost ▶️ View Recording
PsFormatJsonOutputsOnlyJsonToStdout ▶️ View Recording
SecretCrudOnDotNetAppHost ▶️ View Recording
SecretCrudOnTypeScriptAppHost ▶️ View Recording
StagingChannel_ConfigureAndVerifySettings_ThenSwitchChannels ▶️ View Recording
StopAllAppHostsFromAppHostDirectory ▶️ View Recording
StopAllAppHostsFromUnrelatedDirectory ▶️ View Recording
StopNonInteractiveMultipleAppHostsShowsError ▶️ View Recording
StopNonInteractiveSingleAppHost ▶️ View Recording
StopWithNoRunningAppHostExitsSuccessfully ▶️ View Recording
TypeScriptAppHostWithProjectReferenceIntegration ▶️ View Recording

📹 Recordings uploaded automatically from CI run #22690603293

- ExtractSlsaBundleJson now returns 'out bool parseFailed' to distinguish
  invalid JSON (AttestationParseFailed) from missing SLSA provenance
  (SlsaProvenanceNotFound)
- Add explicit JsonObject type checks when iterating attestation array
  elements so non-object values (strings, numbers, null) are safely skipped
- Replace GetProperty/GetString with TryGetProperty + ValueKind checks
  in ExtractProvenanceFromResult to handle wrong-typed predicate values
  without throwing InvalidOperationException
- Fix misleading comment about CertificateExtensionPolicy; the actual
  enforcement is CertificateIdentity.ForGitHubActions SAN/issuer matching
- Fix deeply nested JSON test to use valid JSON structure
- Add tests for non-object array elements, non-string predicateType,
  wrong-typed predicate values, and parseFailed assertions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
var attestations = doc?["attestations"]?.AsArray();
if (attestations is null || attestations.Count == 0)
var attestationsNode = doc?["attestations"];
if (attestationsNode is not JsonArray attestations || attestations.Count == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (attestationsNode is not JsonArray attestations || attestations.Count == 0)
if (attestationsNode is not JsonArray { Count: >0 } attestations)

@DamianEdwards
Copy link
Member

This looks good to me but I'll leave to eng folks to provide approvals.

Use property pattern matching (JsonArray { Count: >0 }) instead of
separate is-check and Count comparison, as suggested by @DamianEdwards.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mitchdenny mitchdenny merged commit 2152012 into release/13.2 Mar 5, 2026
760 of 763 checks passed
@mitchdenny mitchdenny deleted the sigstore-attestation-apis branch March 5, 2026 00:39
@dotnet-policy-service dotnet-policy-service bot added this to the 13.2 milestone Mar 5, 2026
eerhardt pushed a commit to eerhardt/aspire that referenced this pull request Mar 7, 2026
…ft#14931)

* refactor: use Sigstore attestation helper APIs for provenance checking

Replace manual JSON parsing in SigstoreNpmProvenanceChecker with the
new attestation APIs already available in Sigstore 0.3.0:

- Remove ParseAttestation (~80 lines) and ParseProvenanceFromStatement
  (~20 lines) methods and NpmAttestationParseResult class
- Add ExtractSlsaBundleJson to extract bundle JSON from npm response
- Add ExtractProvenanceFromResult using VerificationResult.Statement
  (InTotoStatement) and VerifiedIdentity.Extensions (FulcioCertificateExtensions)
- Use bundle.DsseEnvelope.Payload directly instead of manual base64 decode
- Prefer certificate extensions (cryptographically bound) over SLSA
  predicate values for source repository and ref
- Fix JsonArray safety: use pattern matching instead of AsArray()

Add comprehensive adversarial/security tests (68 total):
- Malformed JSON attacks (deeply nested, truncated, wrong types, null)
- Attestation structure manipulation (swapped predicates, mixed arrays)
- Provenance spoofing (homoglyph URLs, path traversal, null bytes)
- Build type spoofing (query params, wrong domains, empty values)
- URL parsing edge cases (subdomain attacks, non-absolute URIs)
- WorkflowRefInfo parsing edge cases (path traversal in names)
- Statement extraction edge cases (missing fields, empty predicates)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: remove non-Sigstore NpmProvenanceChecker codepath

Delete NpmProvenanceChecker and its tests — production code only uses
SigstoreNpmProvenanceChecker (registered in DI), so the non-Sigstore
fallback was dead code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address PR review: type-safe JSON handling and richer error reporting

- ExtractSlsaBundleJson now returns 'out bool parseFailed' to distinguish
  invalid JSON (AttestationParseFailed) from missing SLSA provenance
  (SlsaProvenanceNotFound)
- Add explicit JsonObject type checks when iterating attestation array
  elements so non-object values (strings, numbers, null) are safely skipped
- Replace GetProperty/GetString with TryGetProperty + ValueKind checks
  in ExtractProvenanceFromResult to handle wrong-typed predicate values
  without throwing InvalidOperationException
- Fix misleading comment about CertificateExtensionPolicy; the actual
  enforcement is CertificateIdentity.ForGitHubActions SAN/issuer matching
- Fix deeply nested JSON test to use valid JSON structure
- Add tests for non-object array elements, non-string predicateType,
  wrong-typed predicate values, and parseFailed assertions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Apply pattern matching suggestion for JsonArray count check

Use property pattern matching (JsonArray { Count: >0 }) instead of
separate is-check and Count comparison, as suggested by @DamianEdwards.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Mitch Denny <mitch@mitchdeny.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI pushed a commit that referenced this pull request Mar 10, 2026
* refactor: use Sigstore attestation helper APIs for provenance checking

Replace manual JSON parsing in SigstoreNpmProvenanceChecker with the
new attestation APIs already available in Sigstore 0.3.0:

- Remove ParseAttestation (~80 lines) and ParseProvenanceFromStatement
  (~20 lines) methods and NpmAttestationParseResult class
- Add ExtractSlsaBundleJson to extract bundle JSON from npm response
- Add ExtractProvenanceFromResult using VerificationResult.Statement
  (InTotoStatement) and VerifiedIdentity.Extensions (FulcioCertificateExtensions)
- Use bundle.DsseEnvelope.Payload directly instead of manual base64 decode
- Prefer certificate extensions (cryptographically bound) over SLSA
  predicate values for source repository and ref
- Fix JsonArray safety: use pattern matching instead of AsArray()

Add comprehensive adversarial/security tests (68 total):
- Malformed JSON attacks (deeply nested, truncated, wrong types, null)
- Attestation structure manipulation (swapped predicates, mixed arrays)
- Provenance spoofing (homoglyph URLs, path traversal, null bytes)
- Build type spoofing (query params, wrong domains, empty values)
- URL parsing edge cases (subdomain attacks, non-absolute URIs)
- WorkflowRefInfo parsing edge cases (path traversal in names)
- Statement extraction edge cases (missing fields, empty predicates)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* chore: remove non-Sigstore NpmProvenanceChecker codepath

Delete NpmProvenanceChecker and its tests — production code only uses
SigstoreNpmProvenanceChecker (registered in DI), so the non-Sigstore
fallback was dead code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address PR review: type-safe JSON handling and richer error reporting

- ExtractSlsaBundleJson now returns 'out bool parseFailed' to distinguish
  invalid JSON (AttestationParseFailed) from missing SLSA provenance
  (SlsaProvenanceNotFound)
- Add explicit JsonObject type checks when iterating attestation array
  elements so non-object values (strings, numbers, null) are safely skipped
- Replace GetProperty/GetString with TryGetProperty + ValueKind checks
  in ExtractProvenanceFromResult to handle wrong-typed predicate values
  without throwing InvalidOperationException
- Fix misleading comment about CertificateExtensionPolicy; the actual
  enforcement is CertificateIdentity.ForGitHubActions SAN/issuer matching
- Fix deeply nested JSON test to use valid JSON structure
- Add tests for non-object array elements, non-string predicateType,
  wrong-typed predicate values, and parseFailed assertions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Apply pattern matching suggestion for JsonArray count check

Use property pattern matching (JsonArray { Count: >0 }) instead of
separate is-check and Count comparison, as suggested by @DamianEdwards.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Mitch Denny <mitch@mitchdeny.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants