guildhouse-spire-plugins/AUDIT.md
Tyler King 420a4e2ea0 Remediate all 17 audit findings from AUDIT.md
Critical fixes:
- F-01: SatScope array form support (single pointer → slice with polymorphic JSON)
- F-02: Add governance-intent@guildhouse.dev as 10th Shellstream extension
- F-06: Replace os.Exit(1) stubs with go-plugin Serve() boilerplate in all cmd/
- F-13: Validate SatScope.ResourcePattern is non-empty

High priority:
- F-03: Add normative Accord policy syntax note to credential-governance.md §8.2
- F-04: Replace OID XXXXX placeholder with explicit PEN reference and IANA TODO
- F-05: Document CredentialComposer hook mapping in spec and plugin-types.md
- F-07/F-08: Commit CI pipeline (.github/workflows/ci.yaml)
- F-09: Add hashicorp/go-plugin v1.6.3 to go.mod

Medium priority:
- F-10: Wire sample-ssh-cert-extensions.json fixture into shellstream tests
- F-11: Cross-reference merkle proof depth limit (256 leaves) in governance spec
- F-12: Add YAML format clarification headers to deploy configs
- F-14: Expand README with project status, docs links, and quick-start

Low priority:
- F-15: Standardize "SSH SVID" → "SSH-SVID" terminology across docs
- F-16: Add GovernanceEpochSeconds to PluginConfig and deploy configs
- F-17: Add troubleshooting section to deployment.md, error handling to OIDC docs

Global: Rename all extension keys from @guildhouse.io to @guildhouse.dev

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-18 11:45:33 -05:00

42 KiB
Raw Blame History

Guildhouse SPIRE Plugins — Audit Report

Date: 2026-02-18 Commit: eb9edf5 (Initial scaffolding: specs, plugins, pkg/shellstream) Files: 52 (excluding .git/) Tests: Unverifiable (Go toolchain not available on audit machine) Build Status: Unverifiable (pre-built binaries in bin/ confirm at least one prior successful build) License: Apache 2.0 (Copyright 2024-2026 Guildhouse Cooperative)


Executive Summary

This repository contains three well-structured specifications, one fully-implemented Go package (pkg/shellstream — 319 lines of production code with 551 lines of tests), and scaffolding for four SPIRE plugin binaries and four supporting packages. The specifications are RFC 2119-compliant and compare favorably to existing SPIFFE standards in organization and rigor.

Two critical spec/code mismatches were identified: (1) the sat-scope@guildhouse.io extension spec requires implementations to accept both single-object and array-of-objects JSON forms, but the Go implementation only handles single-object form — a MUST-level violation; (2) governance-intent@guildhouse.io is referenced in the credential governance spec but is not defined in the Shellstream extension registry, the Go constants, or the test fixtures.

Upstream readiness: NOT READY. The repository requires resolution of 2 critical findings, completion of go-plugin boilerplate for all binaries, addition of external dependencies to go.mod, CI pipeline activation, and proto code generation verification before it can be presented to the SPIFFE community. The specifications are close to SIG-Spec submission quality but need the critical inconsistencies resolved first.


Repository Inventory

Category Count Files
Specifications 3 specs/spiffe-ssh-svid.md, specs/shellstream-extensions.md, specs/credential-governance.md
Plugin sources (cmd/) 8 4 plugins × (main.go + plugin.go)
Library packages (pkg/) 11 shellstream/ (3 files, fully implemented), config/ oidc/ governance/ sshcert/ (2 files each, scaffolded)
Proto definitions 4 governance.proto, notary.proto, credentials.proto, ceremony.proto
Documentation 7 docs/architecture.md through docs/testing.md
Deploy configs 3 deploy/spire-server-config.yaml, deploy/spire-agent-config.yaml, deploy/kustomization.yaml
Test fixtures 4 test/fixtures/sample-*.json, test/fixtures/spire-test-config.hcl
Build/config 6 go.mod, Makefile, buf.yaml, buf.gen.yaml, .gitignore, LICENSE
Pre-built binaries 4 bin/{oidc-attestor,ssh-credential-composer,governance-notifier,substrate-keymanager}
Root docs 1 README.md

Specification Review

spiffe-ssh-svid.md

Rating: 90% — Strong, needs minor clarifications

Strengths:

  • Structure matches SPIFFE spec conventions (Abstract, Notational Conventions, numbered sections with TOC).
  • RFC 2119/8174 normative language used correctly throughout. All MUST/SHOULD/MAY keywords properly capitalized.
  • Certificate format precisely defined: key types (Ed25519 REQUIRED, ECDSA P-256 OPTIONAL, RSA MUST NOT), key ID (SPIFFE ID in canonical URI), validity period (30s min, 5m recommended, 1h max), serial number constraints.
  • Issuance flow (Section 4) clearly specifies 6-step workload-agent-server-CA flow with Workload API RPC extensions (FetchSSHSVID, MintSSHSVID).
  • Trust model (Section 5) well-defined: SSH CA public key in SPIRE trust bundle, federation support, TrustedUserCAKeys + AuthorizedPrincipalsFile server-side.
  • Security considerations (Section 8) thorough: attestation trust anchor, SPIFFE ID visibility, mTLS requirement, rate limiting (60 issuances/min), CA key compromise mitigations, clock skew tolerance (60s).
  • Compatibility (Section 9) excellent: OpenSSH 8.0+, Dropbear, libssh, Paramiko, Go crypto/ssh.
  • Self-contained: an implementor unfamiliar with Guildhouse could implement a basic SSH-SVID system from this spec alone.

Gaps:

  • CredentialComposer hook mapping undefined (see F-05). Section 4, line 239 references "the CredentialComposer plugin chain" but does not specify which SPIRE hook is used. SPIRE's CredentialComposer has 5 methods — all X.509/JWT, none SSH-specific.
  • No versioning strategy for future SSH-SVID format changes. Unlike X.509-SVID which has OID-based extension versioning, there is no defined mechanism for evolving the SSH certificate format.
  • Workload API RPC messages defined inline (Section 4.3) and marked as having "full protobuf definitions in the companion ssh-credential-composer plugin specification" — but no such companion spec exists in this repository.

shellstream-extensions.md

Rating: 92% — Excellent, minor issues

Strengths:

  • Complete extension registry: 9 extensions defined with precise format specifications per extension (Sections 6.16.9).
  • Encoding rules (Section 7) unambiguous: hex lowercase only (uppercase MUST be rejected), base64 standard alphabet with padding (RFC 4648 Section 4), JSON compact encoding, UUID lowercase 8-4-4-4-12 format (uppercase MUST be rejected), decimal strings without leading zeros, comma-separated without whitespace.
  • Co-occurrence constraints (Section 8) precisely tabulated: sat-scopesat-hash (bidirectional), ceremony-idceremony-type (bidirectional), merkle-proofmerkle-root (unidirectional), with explicit note that merkle-root MAY appear without merkle-proof.
  • Required extensions clearly identified: tenant-id@guildhouse.io and roles@guildhouse.io always required; missing either makes the certificate invalid.
  • Forward compatibility explicitly addressed (Section 8, line 379): unknown @guildhouse.io extensions MUST be ignored. This enables additive evolution.
  • Security considerations appropriate: extensions not encrypted, MUST NOT embed secrets, merkle proofs provide auditability not authorization, total payload SHOULD NOT exceed 4 KB, extensions not code.
  • Compatibility: fully compatible with OpenSSH PROTOCOL.certkeys, transparent degradation on non-Guildhouse servers.

Gaps:

  • SatScope array form (Section 6.1, lines 152154): "When multiple SAT scopes exist, the value MUST be a JSON array of scope objects. Implementations MUST accept both forms." This creates a polymorphic encoding requirement that the Go implementation does not satisfy (see F-01).
  • Merkle proof depth limit (Section 6.8, lines 286288): Limits to depth 8 (256 leaves) with 1-byte direction encoding. States "multi-byte direction encoding will be specified in a future revision" — this limitation is not cross-referenced in the credential governance spec.
  • 4 KB payload threshold (Section 9, line 423): Uses SHOULD, not MUST. No enforcement guidance for what happens when exceeded.

credential-governance.md

Rating: 85% — Good, has more unresolved items

Strengths:

  • End-to-end governance flow (Section 6) comprehensive: 7-step flow from event interception through intent creation, policy evaluation, ceremony, SAT redemption, envelope construction, and merkle anchoring.
  • Credential event taxonomy (Section 5) complete: issue, rotate, revoke — each with JCS-canonicalized schema and full field definitions.
  • MutationEnvelope construction (Section 7) deterministic: RFC 8785 JCS canonicalization + domain separation (guildhouse.credential.v1: prefix) + SHA-256. This is excellent for auditability.
  • Ceremony classification (Section 8) well-defined: 5 tiers (Autonomous, SelfGrant, SingleApproval, QuorumApproval, EmergencyBreakGlass) with clear intended use cases and TTL-based triggers.
  • Error handling matrix (Section 10) practical: fail-closed for GovernanceService, fail-open for NotaryService (with durable queue retry), fail-safe for missing policy (defaults to SingleApproval).
  • Security considerations (Section 11) address real threats: TOCTOU via SAT TTL (recommended 60s), replay via max_redemptions=1, self-approval rejection for elevated tiers.

Gaps:

  • governance-intent@guildhouse.io phantom extension (line 580): References this as a required SSH certificate extension, but it is NOT in the Shellstream extension registry (see F-02).
  • Accord Policy Specification missing (line 731): References specs/accord-policy.md (forthcoming). Section 8.2 shows an example YAML policy syntax, but no formal grammar or validation rules exist.
  • X.509 governance OID placeholder (line 584): 1.3.6.1.4.1.XXXXX.1.1 — IANA PEN unresolved.
  • NotaryService leaf batching within epochs (Section 6.9): Mentions epoch-based batching but does not specify batching algorithm, epoch duration configuration, or epoch boundary behavior.
  • Proto message references: Spec references SatToken, CreateIntentRequest, SatScopeMsg, etc. inline, but readers must look in /proto/ to find definitions. The spec should either include canonical proto definitions or add explicit cross-references.

Code Review

pkg/shellstream (Fully Implemented)

Files: doc.go (package docs), shellstream.go (319 lines), shellstream_test.go (551 lines) Test-to-code ratio: 1.73:1 (excellent) Dependencies: Standard library only (encoding/base64, encoding/hex, encoding/json, fmt, strconv, strings)

Spec Compliance — Extension Keys:

All 9 extension key constants (lines 1422) match specs/shellstream-extensions.md Section 6 exactly:

Go Constant Value Spec Section
ExtSatScope sat-scope@guildhouse.io 6.1
ExtSatHash sat-hash@guildhouse.io 6.2
ExtTenantID tenant-id@guildhouse.io 6.3
ExtRoles roles@guildhouse.io 6.4
ExtCeremonyID ceremony-id@guildhouse.io 6.5
ExtCeremonyType ceremony-type@guildhouse.io 6.6
ExtMerkleRoot merkle-root@guildhouse.io 6.7
ExtMerkleProof merkle-proof@guildhouse.io 6.8
ExtGovernanceEpoch governance-epoch@guildhouse.io 6.9

Spec Compliance — Encoding:

  • Hex values: lowercase enforced via strings.ToLower comparison in Validate (lines 241, 286) ✓
  • Base64: base64.StdEncoding (standard alphabet with padding, RFC 4648 Section 4) ✓
  • JSON: json.Marshal produces compact output (no spaces, no newlines) ✓
  • UUID: custom isValidUUID (lines 300318) validates 8-4-4-4-12 lowercase hex ✓
  • Governance epoch: strconv.FormatUint(epoch, 10) — decimal, no leading zeros ✓
  • Roles: strings.Join(roles, ",") — comma-separated, no whitespace ✓

Spec Compliance — Validation:

  • Required fields: tenant-id and roles checked (lines 205223) ✓
  • Co-occurrence: sat-scopesat-hash (lines 226231) ✓
  • Co-occurrence: ceremony-idceremony-type (lines 257262) ✓
  • Co-occurrence: merkle-proofmerkle-root (lines 292294) ✓
  • Hash format: 64 lowercase hex validated for both sat-hash and merkle-root
  • Ceremony type: validated against known set of 4 values (line 273) ✓
  • Forward compatibility: Decode ignores unknown extensions (no filter on unrecognized keys) ✓

Design Quality:

  • GovernanceEpoch zero-vs-unset handled via private hasGovernanceEpoch sentinel with WithGovernanceEpoch()/HasGovernanceEpoch() accessors (lines 7285). Correct and idiomatic.
  • Error messages all prefixed with "shellstream:" for grep-friendly diagnostics.
  • Pure functions — no hidden state, no global variables.
  • Encode returns nil map on error (consistent with Go convention).

Test Coverage:

  • Round-trip: full extensions and minimal extensions (lines 37137) ✓
  • Unknown extension handling: verified ignored (lines 139155) ✓
  • All validation error paths tested: missing tenant-id, missing roles, invalid UUID format, sat-scope/sat-hash co-occurrence, sat-hash format (length, case), ceremony co-occurrence, unknown ceremony type, merkle-proof requires root, merkle-root format, empty role names, role names with commas ✓
  • JSON parsing: valid and invalid sat-scope JSON (lines 336376) ✓
  • Base64: valid and invalid merkle-proof base64 (lines 399436) ✓
  • Governance epoch: valid value, zero value (explicitly set), invalid string (lines 438490) ✓
  • Nil input: both Encode(nil) and Validate(nil) tested (lines 492504) ✓
  • UUID helper: 7 test cases including uppercase rejection (lines 522541) ✓

Issues:

  1. CRITICAL (F-01): SatScope is *SatScope — single pointer. Spec requires array-of-objects support.
  2. MEDIUM (F-13): SatScope.ResourcePattern not validated — could be empty string.

Scaffolded Packages

pkg/config (39 lines impl, 21 lines test):

  • PluginConfig struct with 5 fields: GovernanceAddr, CeremonyAddr, NotaryAddr, TrustDomain, ClusterID.
  • Validate() checks only TrustDomain non-empty. Other address fields not validated.
  • LoadFromHCL() is a stub with TODO referencing hashicorp/hcl.
  • Tests cover: empty trust domain rejected, minimal valid config accepted.

pkg/oidc (43 lines impl, 19 lines test):

  • Verifier interface with single Verify(ctx, rawToken) (*Claims, error) method.
  • Claims struct captures: Subject, Issuer, Audience ([]string), Email, Groups ([]string).
  • NewVerifier(cfg Config) validates issuer non-empty, then returns "not yet implemented" error.
  • Good interface design — mockable for testing.

pkg/governance (74 lines impl, 22 lines test):

  • Client struct with 4 stub methods: CreateIntent, RedeemIntent, CreateCeremony, SubmitMerkleLeaf.
  • IntentResult and RedeemResult types align with credential-governance spec intent lifecycle.
  • NewClient(cfg Config) validates GovernanceAddr non-empty.

pkg/sshcert (56 lines impl, 30 lines test):

  • CertRequest struct imports shellstream.ShellstreamExtensions — good cross-package integration.
  • Build(req *CertRequest) validates request non-nil and SPIFFE ID non-empty, then returns "not yet implemented".
  • References golang.org/x/crypto/ssh in TODO comments.

Assessment: All scaffolded packages follow a consistent pattern: config struct + constructor validation + stub methods returning "not yet implemented" errors. Type definitions are reasonable and align with specs. The scaffolding provides a clear skeleton for future implementation.

cmd/ Plugins

All 4 plugins follow identical scaffolding pattern:

Plugin SPIRE Interface main.go plugin.go
oidc-attestor WorkloadAttestor fmt.Fprintln + os.Exit(1) Empty struct, TODO fields
ssh-credential-composer CredentialComposer fmt.Fprintln + os.Exit(1) Empty struct, TODO fields
governance-notifier Notifier fmt.Fprintln + os.Exit(1) Empty struct, TODO fields
substrate-keymanager KeyManager fmt.Fprintln + os.Exit(1) Empty struct, TODO fields

Issues:

  • No hashicorp/go-plugin HandshakeConfig, ServeConfig, or plugin.Serve() calls (see F-06).
  • No SPIRE interface methods implemented — all binaries exit immediately.
  • Good: Each plugin.go has detailed TODO comments describing the intended flow, referencing specs and packages.

Documentation Review

README.md

  • Clear project description: SPIRE plugins + specs for governed SSH via SPIFFE.
  • Specs, plugins, and packages listed with brief descriptions.
  • Build commands documented: make build, test, lint, clean, proto-gen.
  • Gaps: No links to docs/ directory, "scaffolded" not explained, no quick-start example, no CI badges, no contributor guidance.

docs/architecture.md

  • System diagram showing SPIRE Agent/Server, 4 plugins, and Guildhouse gRPC services.
  • 10-step SSH certificate issuance data flow.
  • Package map and proto dependency table.
  • Accurate and consistent with specs.

docs/plugin-types.md

  • SPIRE plugin interfaces documented: WorkloadAttestor, CredentialComposer, Notifier, KeyManager.
  • Method signatures, invocation timing, selector formats.
  • Line 58: Explicitly notes SSH credential composer intercepts ComposeWorkloadX509SVID "and potentially future SSH-specific hooks" — this is the clearest statement of the architectural gap.

docs/oidc-attestation.md

  • 8-step token verification flow. Token discovery via projected volume.
  • Selector output: oidc:sub, oidc:iss, oidc:email, oidc:group.
  • JWKS caching: Cache-Control header or 5-minute default.
  • Gap: No error handling details (OIDC provider unreachable, token invalid, JWKS malformed).

docs/ssh-certificate-flow.md

  • 9-participant sequence diagram, 10-step walkthrough.
  • Shellstream extensions embedded in certificate shown explicitly.
  • Server-side validation 5 steps documented.
  • Gap: Step 5 (Governance Intent) assumes automatic approval; doesn't detail ceremony flow.

docs/governance-integration.md

  • Intent lifecycle diagram, CreateIntent/RedeemIntent RPC signatures.
  • MutationEnvelope construction 4-step process.
  • Error handling matrix matching credential-governance spec.
  • Plugin configuration example.
  • Accurate and consistent with credential-governance spec.

docs/deployment.md

  • Kubernetes deployment instructions with prerequisites (SPIRE v1.9+).
  • Container image: ghcr.io/guildhouse-cooperative/spire-plugins:latest.
  • Kustomize overlay installation, SPIRE Server/Agent HCL config blocks.
  • RBAC requirements, mTLS configuration, health checks.
  • Gap: No troubleshooting section, no version compatibility matrix.

docs/testing.md

  • Test commands, package-level coverage summary.
  • pkg/shellstream described as having 28 tests (actual count appears higher at ~24 test functions).
  • CI pipeline example (GitHub Actions YAML) — not committed as actual workflow file.
  • Gap: Mock server examples are pseudocode. Integration test guidance is conceptual only.

Build & Project Structure

go.mod

module github.com/guildhouse-cooperative/guildhouse-spire-plugins
go 1.23.6

Zero external dependencies listed. This is correct for the current state (only pkg/shellstream is implemented, using standard library only), but will need additions for:

  • github.com/hashicorp/go-plugin — plugin serving
  • github.com/hashicorp/hcl — config parsing
  • google.golang.org/grpc + google.golang.org/protobuf — gRPC clients
  • github.com/coreos/go-oidc or equivalent — OIDC verification
  • golang.org/x/crypto/ssh — SSH certificate operations

Makefile

Targets: build (4 plugins → bin/), test (go test ./...), lint (go vet ./...), clean (rm bin/ + gen/), proto-gen (buf generate). Pattern rule $(BINDIR)/%: cmd/%/*.go is clean and idiomatic.

Directory Structure

Follows Go conventions: cmd/ for binaries, pkg/ for libraries, proto/ for proto definitions, gen/ (gitignored) for generated code. No internal/ directory — acceptable given all packages are designed for external consumption.

.gitignore

Correctly excludes: /bin/, *.exe, *.so, *.dylib, /gen/, .idea/, .vscode/, .DS_Store, /go.work.

CI Readiness

No CI pipeline committed. docs/testing.md contains a GitHub Actions YAML example but it is not in .github/workflows/. The Makefile targets are CI-ready (make build, make test, make lint).


Proto Files

4 proto files from 2 packages, all with copy headers:

File Source Package Go Package
proto/quartermaster/v1/governance.proto services/qm-proto/ quartermaster.v1 gen/quartermaster/v1;quartermasterv1
proto/quartermaster/v1/notary.proto services/qm-proto/ quartermaster.v1 gen/quartermaster/v1;quartermasterv1
proto/quartermaster/v1/credentials.proto services/qm-proto/ quartermaster.v1 gen/quartermaster/v1;quartermasterv1
proto/bascule/v1/ceremony.proto services/bascule-proto/ bascule.v1 gen/bascule/v1;basculev1

Each file has a 3-line header:

// Source of truth: guildhouse monorepo
// services/<path>
// This file is a copy for Go code generation. Do not edit here.

buf.yaml (v2): Module at proto/, depends on buf.build/protocolbuffers/wellknowntypes, STANDARD lint, FILE breaking change detection.

buf.gen.yaml (v2): Generates Go protobuf + gRPC code to gen/ with paths=source_relative.

Proto-to-spec alignment:

  • governance.proto SatScopeMsg (registry_type, verbs, resource_pattern) matches SatScope struct in shellstream.go
  • ceremony.proto ceremony types match validCeremonyTypes map ✓
  • notary.proto CreateAnchorRequest.leaves matches merkle leaf submission in credential-governance spec ✓
  • All protos import google/protobuf/timestamp.proto for time handling ✓

Status: gen/ directory is empty and gitignored. make proto-gen requires buf CLI. No generated code available for import.


Deployment Manifests

deploy/spire-server-config.yaml

YAML-format reference configuration for SPIRE Server. Lists all 3 server-side plugins (guildhouse_substrate, guildhouse_ssh, guildhouse_governance) with plugin_cmd paths and plugin_data configuration.

Note: SPIRE traditionally uses HCL format for configuration. This file uses YAML, which may not be directly usable. The docs/deployment.md shows HCL examples. The relationship between these YAML reference configs and the HCL examples in docs needs clarification (see F-12).

deploy/spire-agent-config.yaml

YAML-format reference configuration for SPIRE Agent. Lists guildhouse_oidc as the only custom agent-side plugin.

deploy/kustomization.yaml

Kustomize overlay patching spire-server and spire-agent Deployments. Uses init containers to copy plugin binaries from ghcr.io/guildhouse-cooperative/spire-plugins:latest into shared emptyDir volumes at /opt/spire/plugins/.

Configuration consistency:

  • Plugin binary paths in deploy configs match Makefile binary names ✓
  • Plugin names (guildhouse_substrate, guildhouse_ssh, guildhouse_governance, guildhouse_oidc) consistent across deploy configs ✓
  • Configuration fields (trust_domain, governance_addr, notary_addr, ceremony_addr, cluster_id) match pkg/config/PluginConfig struct fields ✓

Test Fixtures

Fixture Content Used By
sample-oidc-token.json Mock Keycloak OIDC token (iss, sub, email, groups, tenant_id) No test
sample-sat-scope.json Single SatScope {"registry_type":"oci","verbs":["push","pull"],...} No test
sample-ssh-cert-extensions.json Full 9-extension map + 2 OpenSSH standard extensions No test
spire-test-config.hcl PluginConfig HCL with test trust domain and addresses No test

All 4 fixtures are orphaned — they exist for documentation/reference purposes but are not loaded by any test. The pkg/shellstream tests create fixtures inline in shellstream_test.go. The HCL fixture cannot be used because config.LoadFromHCL is a stub (see F-10).

Fixture accuracy:

  • sample-ssh-cert-extensions.json extension keys match all 9 spec-defined extensions ✓
  • sample-sat-scope.json structure matches SatScope struct ✓
  • sample-oidc-token.json is well-formed with realistic Keycloak claims ✓
  • spire-test-config.hcl fields match PluginConfig struct ✓

Cross-Cutting Issues

Extension Key Consistency

9 of 9 defined extensions are consistent across all sources. 1 phantom extension exists:

Extension Shellstream Spec Go Code Test Fixture Governance Spec
sat-scope@guildhouse.io Section 6.1 ExtSatScope present referenced
sat-hash@guildhouse.io Section 6.2 ExtSatHash present referenced
tenant-id@guildhouse.io Section 6.3 ExtTenantID present referenced
roles@guildhouse.io Section 6.4 ExtRoles present referenced
ceremony-id@guildhouse.io Section 6.5 ExtCeremonyID present referenced
ceremony-type@guildhouse.io Section 6.6 ExtCeremonyType present referenced
merkle-root@guildhouse.io Section 6.7 ExtMerkleRoot present referenced
merkle-proof@guildhouse.io Section 6.8 ExtMerkleProof present referenced
governance-epoch@guildhouse.io Section 6.9 ExtGovernanceEpoch present referenced
governance-intent@guildhouse.io NOT DEFINED NOT DEFINED NOT PRESENT line 580

Security Review

  • No hardcoded secrets in any file. Test fixtures are clearly marked as samples.
  • OIDC verifier stub has correct security skeleton (issuer validation in constructor).
  • Proto files use google.protobuf.Timestamp rather than raw epoch seconds (better for safety).
  • SSH certificate security is addressed comprehensively in spiffe-ssh-svid.md Section 8: short-lived certs as primary revocation mechanism, CA key protection via HSM, mTLS for agent-server communication, workload API socket protection, rate limiting.
  • Governance-optional model is safe: credential issuance works without the governance notifier — governance adds auditability, not gating. The governance-notifier plugin is a SPIRE Notifier (receives events after the fact), not a gatekeeper. This is a deliberate and correct design choice.
  • MutationEnvelope determinism (RFC 8785 JCS + domain separation) is cryptographically sound for audit trail integrity.

Upstream Readiness Assessment

Criterion Status Blocker?
Critical spec/code mismatches resolved NO (F-01, F-02) YES
All packages compile UNVERIFIED YES
All tests pass UNVERIFIED YES
Plugins loadable by SPIRE NO (F-06) YES
External dependencies in go.mod NO (zero deps) YES
Proto codegen pipeline verified UNVERIFIED YES
CI pipeline active NO YES
Accord policy spec exists NO (F-03) No (deferrable)
X.509 OID registered NO (F-04) No (SSH path unaffected)
Documentation complete PARTIAL No

Minimum viable path to upstream readiness:

  1. Fix F-01 (SatScope array form support in Go code)
  2. Resolve F-02 (governance-intent extension — add to shellstream spec or remove from governance spec)
  3. Add go-plugin boilerplate to all cmd/*/main.go
  4. Add external dependencies to go.mod
  5. Verify proto code generation with buf
  6. Commit and activate CI pipeline (GitHub Actions)
  7. Verify build + tests pass in CI

SIG-Spec submission readiness: The SSH-SVID spec is close to submission quality. It would benefit from:

  • Clarifying the CredentialComposer hook mapping (whether upstream SPIRE changes are needed)
  • Resolving the governance-intent extension cross-reference
  • Adding a versioning/evolution strategy section
  • Having at least one working reference implementation (the shellstream package qualifies, but the full plugin chain should be demonstrable)

Critical Issues

F-01: SatScope Single-vs-Array Form (CRITICAL)

Location: pkg/shellstream/shellstream.go lines 3741 (SatScope struct), 46 (field type), 100107 (Encode), 147153 (Decode) Spec: specs/shellstream-extensions.md lines 137154 (Section 6.1)

The spec states at lines 152154: "When a single SAT scope exists, the value MUST be a single JSON object. When multiple SAT scopes exist, the value MUST be a JSON array of scope objects. Implementations MUST accept both forms."

The Go code defines SatScope *SatScope (single pointer). Encode() at line 101 calls json.Marshal(ext.SatScope) — always producing a single JSON object. Decode() at line 149 calls json.Unmarshal([]byte(v), scope) into a single &SatScope{} — which will fail on array input.

Impact: Any certificate with multiple SAT scopes will fail to decode. Two independent implementors — one using single form, one using array form — will produce incompatible results.

Recommendation: Change the field to support both forms. Options:

  1. Change to SatScopes []*SatScope with encoding logic that produces a single object when len == 1 and an array when len > 1.
  2. Implement custom unmarshal that detects [ at byte 0 to distinguish forms.
  3. Add test cases for both {"registry_type":...} and [{"registry_type":...},{"registry_type":...}] inputs.

F-02: governance-intent@guildhouse.io Cross-Spec Inconsistency (CRITICAL)

Location: specs/credential-governance.md line 580 (Section 9.2 table) Cross-reference: specs/shellstream-extensions.md Section 6 (only 9 extensions defined), pkg/shellstream/shellstream.go lines 1322 (9 constants)

The credential governance spec's Section 9.2 lists governance-intent@guildhouse.io in the SSH certificate extensions table with description "The intent_id from the governance flow." This extension is not defined anywhere else: not in the Shellstream extension registry, not in the Go constants, not in the test fixtures.

Impact: Implementors following the governance spec will embed an extension with no defined format, encoding rules, or validation constraints. Implementors following the shellstream spec will not include it.

Recommendation: Either:

  1. Add governance-intent@guildhouse.io to specs/shellstream-extensions.md Section 6 with full format definition (likely: string containing intent UUID), and add ExtGovernanceIntent constant + encode/decode/validate support to shellstream.go.
  2. Remove the reference from specs/credential-governance.md line 580 if the intent ID should not be embedded in certificates.

High Priority

F-03: Accord Policy Specification Missing

Location: specs/credential-governance.md line 731 Description: References specs/accord-policy.md (forthcoming) but no such file exists. Section 8.2 (line 438) shows an example YAML policy syntax, but there is no formal grammar, validation schema, or evaluation semantics. Impact: Operators cannot write or validate Accord policies. The example in Section 8.2 is illustrative but not normative. Recommendation: Either write specs/accord-policy.md or add a normative note to Section 8.2 making the example syntax authoritative until the standalone spec is published.

F-04: X.509 Governance OID Placeholder

Location: specs/credential-governance.md line 584 Description: Custom OID 1.3.6.1.4.1.XXXXX.1.1 — the XXXXX is an unresolved IANA Private Enterprise Number. Impact: X.509 SVID governance embedding cannot be implemented. Recommendation: Register a PEN with IANA or use an OID arc already assigned to Guildhouse Cooperative. Replace XXXXX with the actual number.

F-05: CredentialComposer Hook Mapping Unclear

Location: specs/spiffe-ssh-svid.md line 239, docs/plugin-types.md line 58 Description: SPIRE's CredentialComposer interface has 5 methods (ComposeServerX509CA, ComposeServerX509SVID, ComposeAgentX509SVID, ComposeWorkloadX509SVID, ComposeWorkloadJWTSVID) — all X.509/JWT, none SSH-specific. docs/plugin-types.md line 58 states the SSH credential composer "Intercepts ComposeWorkloadX509SVID (and potentially future SSH-specific hooks)." This means either SSH is piggybacked onto X.509 flow (architecturally questionable) or upstream SPIRE changes are needed. Impact: Unclear how to implement the plugin against SPIRE's actual interface. SPIFFE SIG-Spec reviewers will ask about this. Recommendation: Document the exact hook mapping explicitly in the SSH-SVID spec. If upstream SPIRE changes are needed (a new ComposeWorkloadSSHSVID method), file an issue on spiffe/spire and reference it in the spec.

F-06: No go-plugin Serving Pattern

Location: All cmd/*/main.go files (4 files) Description: Every plugin main.go prints "not yet implemented" and calls os.Exit(1). None contain hashicorp/go-plugin HandshakeConfig, ServeConfig, or plugin.Serve() calls. Impact: SPIRE cannot load any plugin. The binaries are non-functional. Recommendation: Add go-plugin boilerplate with HandshakeConfig and stub GRPCServer that returns "unimplemented" gRPC errors. This allows SPIRE to load the plugin and report method-level errors rather than a process crash.

F-07: No Generated Proto Code

Location: buf.yaml, buf.gen.yaml, .gitignore (excludes /gen/) Description: buf configuration is correct. The gen/ directory is gitignored. Running make proto-gen requires buf CLI. No generated Go code exists in the repository. Impact: Any package that needs proto types must run make proto-gen first. go get will not work for consumers. Recommendation: Either commit generated code (common in Go projects) or document buf as a hard prerequisite and add it to CI.

F-08: Build/Test Unverifiable

Description: Go toolchain not available on audit machine. Pre-built binaries in bin/ are statically-linked ELF x86-64 Go executables, confirming at least one prior successful build. However, current compile status and test results cannot be verified. Impact: Cannot confirm code compiles cleanly or tests pass. Recommendation: Commit a CI pipeline (the example in docs/testing.md lines 97112 is a starting point). Once CI is green with build + test + vet, this finding is resolved.

F-09: go.mod Has Zero External Dependencies

Location: go.mod (2 lines total) Description: Module declaration and Go version only. No require block. TODO comments across scaffolded packages reference: hashicorp/go-plugin, hashicorp/hcl, coreos/go-oidc, google.golang.org/grpc, google.golang.org/protobuf, golang.org/x/crypto/ssh. Impact: None of the scaffolded packages can be implemented without adding dependencies. The current zero-dep state is correct for what's implemented but must change as development continues. Recommendation: Add dependencies as they become needed. Consider pinning SPIRE SDK version to ensure compatibility.


Medium Priority

F-10: Test Fixtures Orphaned

Location: test/fixtures/ (4 files) Description: None of the 4 fixture files are loaded by any test. pkg/shellstream tests create all test data inline. spire-test-config.hcl cannot be used because config.LoadFromHCL() is a stub. Impact: Fixtures may drift from code/spec as the project evolves. Orphaned files add maintenance burden. Recommendation: Either add tests that load fixtures (the docs/testing.md shows a loadFixture pattern) or remove them and rely on inline test data.

F-11: Merkle Proof Depth Limit Not Cross-Referenced

Location: specs/shellstream-extensions.md lines 286288 (Section 6.8) Description: Proof encoding limits depth to 8 (256 leaves per epoch) via 1-byte direction byte. This limitation is not mentioned in specs/credential-governance.md. Impact: Deployments with > 256 credential events per governance epoch will produce proofs that cannot be encoded. Recommendation: Add a cross-reference in specs/credential-governance.md Section 9.1 noting the shellstream proof depth limit and its implications for epoch duration configuration.

F-12: Deploy Config Format Ambiguity

Location: deploy/spire-server-config.yaml, deploy/spire-agent-config.yaml Description: Files use .yaml extension and YAML syntax (key: value), but SPIRE's native configuration format is HCL. The docs/deployment.md shows HCL-format examples. It's unclear whether these YAML configs are directly usable by SPIRE or are reference documents requiring conversion. Impact: Operators may attempt to use these files directly with SPIRE and encounter configuration errors. Recommendation: Add a header comment clarifying the purpose of these files. If they're reference docs, say so. If SPIRE supports YAML config (some versions do), document the minimum SPIRE version. Consider providing HCL versions alongside.

F-13: SatScope.ResourcePattern Not Validated

Location: pkg/shellstream/shellstream.go lines 247254 (Validate checks RegistryType and Verbs but not ResourcePattern) Description: An empty ResourcePattern passes validation. While the spec does not explicitly mark it REQUIRED with RFC 2119 language, an empty pattern is semantically meaningless ("authorized to do X on nothing"). Impact: A certificate could contain a SatScope with no resource pattern, which is likely a bug. Recommendation: Either add ResourcePattern != "" validation or add an explicit note in the spec clarifying that empty patterns are valid (matching nothing).

F-14: README Gaps

Location: README.md Description: No links to docs/ directory. No explanation of what "scaffolded" means. No quick-start example. No CI badges. No contributor guidance or code of conduct link. Impact: First-time visitors lack orientation. Recommendation: Add a "Project Status" section explaining the scaffolding stage, link to docs/, add a quick-start section, and prepare for CI badges.


Low Priority

F-15: Terminology Inconsistency

Description: The hyphenated form "SSH-SVID" is used in specs/spiffe-ssh-svid.md (title and throughout). Some docs use "SSH SVID" (space) or "SSH certificate" interchangeably. Recommendation: Standardize on "SSH-SVID" as the canonical term (matching the spec title) and grep-replace inconsistencies.

F-16: Governance Epoch Configurability Undefined

Location: specs/credential-governance.md (describes epochs as "deployment-configurable") Description: No configuration example, no pkg/config field, no deploy manifest entry for epoch duration. Recommendation: Add governance_epoch_seconds or similar to PluginConfig and deploy config examples.

F-17: Documentation Gaps

Locations:

  • docs/deployment.md: No troubleshooting section.
  • docs/oidc-attestation.md: No error handling details (OIDC provider unreachable, token expired, JWKS malformed).
  • docs/testing.md: Mock server examples are pseudocode (not runnable). CI pipeline example not committed as actual workflow file. Recommendation: Add troubleshooting sections and convert pseudocode to real examples as implementation progresses.

Appendix A: Extension Key Registry

# Extension Key Format Presence Spec Section Go Constant Test Fixture Governance Spec
1 sat-scope@guildhouse.io JSON object or array REQUIRED (when SAT present) shellstream 6.1 ExtSatScope present referenced
2 sat-hash@guildhouse.io SHA-256 hex, 64 chars, lowercase REQUIRED (when SAT present) shellstream 6.2 ExtSatHash present referenced
3 tenant-id@guildhouse.io UUID, lowercase, 8-4-4-4-12 REQUIRED shellstream 6.3 ExtTenantID present referenced
4 roles@guildhouse.io Comma-separated, no whitespace REQUIRED shellstream 6.4 ExtRoles present referenced
5 ceremony-id@guildhouse.io UUID, lowercase, 8-4-4-4-12 OPTIONAL shellstream 6.5 ExtCeremonyID present referenced
6 ceremony-type@guildhouse.io Enum: self_grant, single_approval, quorum_approval, emergency_break_glass OPTIONAL (requires ceremony-id) shellstream 6.6 ExtCeremonyType present referenced
7 merkle-root@guildhouse.io SHA-256 hex, 64 chars, lowercase OPTIONAL shellstream 6.7 ExtMerkleRoot present referenced
8 merkle-proof@guildhouse.io Base64, standard alphabet, padded OPTIONAL (requires merkle-root) shellstream 6.8 ExtMerkleProof present referenced
9 governance-epoch@guildhouse.io Decimal uint64, no leading zeros OPTIONAL shellstream 6.9 ExtGovernanceEpoch present referenced
10 governance-intent@guildhouse.io Undefined Undefined NOT IN SPEC NOT IN CODE NOT IN FIXTURE line 580

Appendix B: Spec Terminology Glossary

Term Definition Source Notes
SSH-SVID specs/spiffe-ssh-svid.md Section 1 OpenSSH certificate encoding a SPIFFE ID as principal identity
SPIFFE ID specs/spiffe-ssh-svid.md Section 1 spiffe://<trust-domain>/<path> URI format
Shellstream Extension specs/shellstream-extensions.md Section 3 SSH certificate extension with @guildhouse.io vendor suffix
SAT (Substrate Attestation Token) specs/shellstream-extensions.md Section 3 Signed token binding SPIFFE identity to permitted operations
SatScope specs/shellstream-extensions.md Section 3 Registry type + verbs + resource pattern
Credential Event specs/credential-governance.md Section 3 Discrete lifecycle operation (issue, rotate, revoke)
MutationIntent specs/credential-governance.md Section 3 Authorization request with state machine
MutationEnvelope specs/credential-governance.md Section 3 Signed wrapper around canonicalized governance state change
Governance Epoch specs/credential-governance.md Section 3 Time-bounded interval for merkle leaf accumulation
Merkle Anchor specs/credential-governance.md Section 3 Immutable record created by NotaryService
Accord Policy specs/shellstream-extensions.md Section 3 OPA-backed tenant-scoped authorization policy
Governance Ceremony specs/credential-governance.md Section 3 Multi-stakeholder approval workflow
Trust Domain specs/spiffe-ssh-svid.md Section 1 SPIFFE trust boundary
Trust Bundle specs/spiffe-ssh-svid.md Section 5 Collection of CA public keys for a trust domain
CredentialComposer docs/plugin-types.md line 42 SPIRE plugin interface for customizing credential fields

Appendix C: Plugin Interface Compliance Matrix

Plugin SPIRE Interface Required Methods Implemented Stubbed Status
oidc-attestor WorkloadAttestor Attest(ctx, pid) 0 0 Not started — binary exits immediately
ssh-credential-composer CredentialComposer ComposeServerX509CA, ComposeServerX509SVID, ComposeAgentX509SVID, ComposeWorkloadX509SVID, ComposeWorkloadJWTSVID 0 0 Not started — binary exits immediately
governance-notifier Notifier Notify, NotifyAndAdvise 0 0 Not started — binary exits immediately
substrate-keymanager KeyManager GenerateKey, GetPublicKey, GetPublicKeys, SignData 0 0 Not started — binary exits immediately

Total: 0 of 12 interface methods implemented across all 4 plugins.

Note: The plugin.go files in each cmd/ directory define empty structs with detailed TODO comments describing the intended implementation flow. The struct types exist but have no methods attached. No hashicorp/go-plugin integration code exists in any plugin.