Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions pkg/skills/lockfile/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,47 @@ type Entry struct {
// ContentDigest is a deterministic SHA-256 dirhash of the materialized
// skill file set, used for on-disk integrity verification.
ContentDigest string `yaml:"contentDigest,omitempty"`
// Provenance records the verified signer identity of the installed
// artifact — the trust-on-first-use anchor later installs, syncs, and
// upgrades are checked against. Nil for entries recorded before
// verification existed and for unsigned entries (see Unsigned); an
// entry never carries both.
Provenance *Provenance `yaml:"provenance,omitempty"`
// Unsigned is true when the artifact carried no signature and the user
// explicitly accepted that with --allow-unsigned. It is a recorded
// trust decision, not an omission: replacing an unsigned entry with a
// signed one clears it, and a signed entry never becomes unsigned
// without the same explicit flag.
Unsigned bool `yaml:"unsigned,omitempty"`
// RequiredBy lists parent skill names for transitively materialized
// dependencies (skills declared via toolhive.requires).
RequiredBy []string `yaml:"requiredBy,omitempty"`
// Explicit is true when the user directly installed this skill; explicit
// entries are exempt from cascade removal when RequiredBy becomes empty.
Explicit bool `yaml:"explicit,omitempty"`
// Extra round-trips fields this binary does not know about (e.g. the
// Sigstore provenance fields a future schema adds under version 1), so a
// Extra round-trips fields this binary does not know about, so a
// Load→modify→Save cycle by an older binary preserves rather than strips
// them. It applies per entry: an entry this binary rewrites (reinstall,
// upgrade) is built fresh, so its unknown fields — which described the
// previous install — are intentionally dropped along with it.
Extra map[string]any `yaml:",inline"`
}

// Provenance is the Sigstore signer identity recorded for a verified entry.
type Provenance struct {
// SignerIdentity is the certificate subject identity: for GitHub
// Actions certificates, the workflow path relative to the repository;
// otherwise the certificate SAN verbatim (a URI, email, or SPIFFE ID).
SignerIdentity string `yaml:"signerIdentity"`
// CertIssuer is the OIDC issuer that authenticated the signer.
CertIssuer string `yaml:"certIssuer"`
// RepositoryURI is the source repository from the Fulcio certificate
// extensions, when present.
RepositoryURI string `yaml:"repositoryUri,omitempty"`
// SigstoreURL is the Sigstore instance the signature chains to.
SigstoreURL string `yaml:"sigstoreUrl,omitempty"`
}

// Lockfile is the parsed contents of a project's toolhive.lock.yaml.
type Lockfile struct {
// Version is the lock file schema version.
Expand Down
99 changes: 94 additions & 5 deletions pkg/skills/lockfile/lockfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ func TestUnknownFieldsSurviveLoadModifySave(t *testing.T) {
" - name: signed-skill\n" +
" source: ghcr.io/org/signed-skill:1.0.0\n" +
" digest: " + ociDigest(1) + "\n" +
" provenance:\n" +
" signerIdentity: dev@example.com\n" +
" certIssuer: https://accounts.example.com\n"
" attestations:\n" +
" - predicateType: https://slsa.dev/provenance/v1\n" +
" digest: " + ociDigest(3) + "\n"
require.NoError(t, os.WriteFile(path, []byte(futureLock), 0o644))

// Load, touch an unrelated entry, save — the classic older-binary write.
Expand All @@ -152,14 +152,14 @@ func TestUnknownFieldsSurviveLoadModifySave(t *testing.T) {
data, err := os.ReadFile(path) //nolint:gosec // fixed test path
require.NoError(t, err)
saved := string(data)
assert.Contains(t, saved, "signerIdentity: dev@example.com", "unknown entry fields must survive a Load->Save cycle")
assert.Contains(t, saved, "predicateType: https://slsa.dev/provenance/v1", "unknown entry fields must survive a Load->Save cycle")
assert.Contains(t, saved, "futureTopLevelField: keep-me", "unknown top-level fields must survive a Load->Save cycle")

loaded, err := Load(root)
require.NoError(t, err)
signed, ok := loaded.Get("signed-skill")
require.True(t, ok)
assert.Contains(t, signed.Extra, "provenance")
assert.Contains(t, signed.Extra, "attestations")
}

func TestSaveRejectsInvalidLockfile(t *testing.T) {
Expand Down Expand Up @@ -385,3 +385,92 @@ func TestConcurrentUpsertEntryDoesNotLoseUpdates(t *testing.T) {
func skillNameForIndex(i int) string {
return "skill-" + string(rune('a'+i/26)) + string(rune('a'+i%26))
}

// TestProvenanceRoundTrip covers the Stack-2 schema: a verified entry's
// provenance block and an unsigned entry's exception flag both survive
// Save -> Load intact under schema version 1.
func TestProvenanceRoundTrip(t *testing.T) {
t.Parallel()
root := testRoot(t)

lf := &Lockfile{Version: CurrentVersion}
signed := Entry{
Name: "signed-skill",
Source: "ghcr.io/org/signed-skill",
ResolvedReference: "ghcr.io/org/signed-skill:latest",
Digest: ociDigest(1),
Provenance: &Provenance{
SignerIdentity: "/.github/workflows/release.yml",
CertIssuer: "https://token.actions.githubusercontent.com",
RepositoryURI: "https://github.com/org/signed-skill",
SigstoreURL: "https://rekor.sigstore.dev",
},
Explicit: true,
}
unsigned := Entry{
Name: "unsigned-skill",
Source: "unsigned-skill",
Digest: ociDigest(2),
Unsigned: true,
Explicit: true,
}
lf.Upsert(signed)
lf.Upsert(unsigned)
require.NoError(t, lf.Save(root))

loaded, err := Load(root)
require.NoError(t, err)

gotSigned, ok := loaded.Get("signed-skill")
require.True(t, ok)
assert.Equal(t, signed.Provenance, gotSigned.Provenance)
assert.False(t, gotSigned.Unsigned)
assert.Empty(t, gotSigned.Extra, "a typed provenance block must not leak into the inline Extra map")

gotUnsigned, ok := loaded.Get("unsigned-skill")
require.True(t, ok)
assert.True(t, gotUnsigned.Unsigned)
assert.Nil(t, gotUnsigned.Provenance)
}

// TestProvenanceGraduatesFromExtraMap: before this schema change, a
// provenance block written by a newer binary round-tripped through the
// Extra inline map. Now that the field is typed, loading such a file must
// parse it into Entry.Provenance — not duplicate it in Extra, which would
// make Save produce a colliding key.
func TestProvenanceGraduatesFromExtraMap(t *testing.T) {
t.Parallel()
root := testRoot(t)
path, err := root.Path()
require.NoError(t, err)

handWritten := "" +
"version: 1\n" +
"skills:\n" +
" - name: signed-skill\n" +
" source: ghcr.io/org/signed-skill\n" +
" digest: " + ociDigest(1) + "\n" +
" provenance:\n" +
" signerIdentity: dev@example.com\n" +
" certIssuer: https://accounts.example.com\n" +
" explicit: true\n"
require.NoError(t, os.WriteFile(path, []byte(handWritten), 0o644))

loaded, err := Load(root)
require.NoError(t, err)
entry, ok := loaded.Get("signed-skill")
require.True(t, ok)
require.NotNil(t, entry.Provenance, "provenance must parse into the typed field")
assert.Equal(t, "dev@example.com", entry.Provenance.SignerIdentity)
assert.NotContains(t, entry.Extra, "provenance", "the typed field must not also appear in Extra")

// A Save after modification must not produce duplicate keys.
require.NoError(t, UpsertEntry(root, Entry{
Name: "other-skill", Source: "other-skill", Digest: ociDigest(2),
}))
reloaded, err := Load(root)
require.NoError(t, err)
entry, ok = reloaded.Get("signed-skill")
require.True(t, ok)
assert.Equal(t, "dev@example.com", entry.Provenance.SignerIdentity)
}
46 changes: 46 additions & 0 deletions pkg/skills/lockfile/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,52 @@ func validateEntry(entry Entry) error {
return fmt.Errorf("entry %q: resolvedReference: %w", entry.Name, err)
}
}
if entry.Provenance != nil && entry.Unsigned {
return fmt.Errorf("entry %q: provenance and unsigned are mutually exclusive"+
" — an entry is either a verified signature or a recorded unsigned exception", entry.Name)
}
if entry.Provenance != nil {
if err := validateProvenance(entry.Provenance); err != nil {
return fmt.Errorf("entry %q: provenance: %w", entry.Name, err)
}
}
return nil
}

// validateProvenance syntactically constrains a provenance block. Like
// resolvedReference, these values are hand-editable and feed the identity
// policy that future verifications are checked against, so they must be
// well-formed graphic strings of bounded length. Validation is purely
// syntactic — whether the identity is trustworthy is the verifier's job.
func validateProvenance(p *Provenance) error {
if p.SignerIdentity == "" {
return errors.New("signerIdentity is required")
}
if p.CertIssuer == "" {
return errors.New("certIssuer is required")
}
fields := map[string]string{
"signerIdentity": p.SignerIdentity,
"certIssuer": p.CertIssuer,
"repositoryUri": p.RepositoryURI,
"sigstoreUrl": p.SigstoreURL,
}
for name, value := range fields {
if value == "" {
continue
}
if len(value) > maxReferenceLength {
return fmt.Errorf("%s exceeds %d characters", name, maxReferenceLength)
}
if strings.TrimSpace(value) != value {
return fmt.Errorf("%s has leading or trailing whitespace", name)
}
for _, r := range value {
if !unicode.IsGraphic(r) || unicode.IsSpace(r) {
return fmt.Errorf("%s contains non-graphic or whitespace character %q", name, r)
}
}
}
return nil
}

Expand Down
63 changes: 63 additions & 0 deletions pkg/skills/lockfile/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,69 @@ func TestValidateLockfile(t *testing.T) {
}},
wantErr: "requiredBy cycle",
},
{
name: "valid provenance",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "signed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
SignerIdentity: "dev@example.com",
CertIssuer: "https://accounts.example.com",
}},
}},
},
{
name: "provenance and unsigned are mutually exclusive",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "conflicted", Source: "s", Digest: validSHA256Digest, Unsigned: true, Provenance: &Provenance{
SignerIdentity: "dev@example.com",
CertIssuer: "https://accounts.example.com",
}},
}},
wantErr: "mutually exclusive",
},
{
name: "provenance missing signer identity",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "signed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
CertIssuer: "https://accounts.example.com",
}},
}},
wantErr: "signerIdentity is required",
},
{
name: "provenance missing cert issuer",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "signed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
SignerIdentity: "dev@example.com",
}},
}},
wantErr: "certIssuer is required",
},
{
name: "provenance with control characters rejected",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "signed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
SignerIdentity: "dev@example.com\x1b[31m",
CertIssuer: "https://accounts.example.com",
}},
}},
wantErr: "non-graphic",
},
{
name: "provenance field too long rejected",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "signed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
SignerIdentity: strings.Repeat("a", maxReferenceLength+1),
CertIssuer: "https://accounts.example.com",
}},
}},
wantErr: "exceeds",
},
{
name: "unsigned exception alone is valid",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
{Name: "unsigned", Source: "s", Digest: validSHA256Digest, Unsigned: true},
}},
},
{
name: "requiredBy diamond is not a cycle",
lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
Expand Down
30 changes: 30 additions & 0 deletions pkg/skills/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ type InstallOptions struct {
ProjectRoot string `json:"project_root,omitempty"`
// Group is the group name to add the skill to after installation.
Group string `json:"group,omitempty"`
// AllowUnsigned permits installing a project-scoped skill whose artifact
// carries no Sigstore signature. Without it, unsigned artifacts are
// rejected; with it, the lock entry records the exception as
// "unsigned: true". Skill content is AI-executed instructions, so this
// is an explicit per-install trust decision, never a default.
AllowUnsigned bool `json:"allow_unsigned,omitempty"`
// LayerData is the tar.gz content from an OCI layer. Internal use only — NOT exposed via HTTP API.
LayerData []byte `json:"-"`
// Reference is the full OCI reference (e.g. ghcr.io/org/skill:v1).
Expand Down Expand Up @@ -70,6 +76,30 @@ type InstallOptions struct {
// normal "same digest means content is already correct" fast path must
// not apply. Internal use only — NOT exposed via HTTP API.
SyncRestore bool `json:"-"`
// Provenance carries the verified signer identity established during
// install-time verification, for recording into the lock entry. Set by
// the verification step, nil when the artifact is unsigned or
// verification did not run. Internal use only — NOT exposed via HTTP API.
Provenance *ProvenanceInfo `json:"-"`
// SigstoreBundle is the serialized Sigstore bundle backing Provenance,
// persisted alongside the install record so sync can re-verify offline.
// Internal use only — NOT exposed via HTTP API.
SigstoreBundle []byte `json:"-"`
}

// ProvenanceInfo is the verified signer identity of an installed artifact,
// the in-memory mirror of the lock file's provenance block.
type ProvenanceInfo struct {
// SignerIdentity is the certificate subject identity (workflow path for
// GitHub Actions certificates, SAN verbatim otherwise).
SignerIdentity string `json:"-"`
// CertIssuer is the OIDC issuer that authenticated the signer.
CertIssuer string `json:"-"`
// RepositoryURI is the source repository from the certificate
// extensions, when present.
RepositoryURI string `json:"-"`
// SigstoreURL is the Sigstore instance the signature chains to.
SigstoreURL string `json:"-"`
}

// InstallResult contains the outcome of an Install operation.
Expand Down
Loading