# 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.1–6.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-scope` ↔ `sat-hash` (bidirectional), `ceremony-id` ↔ `ceremony-type` (bidirectional), `merkle-proof` → `merkle-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 152–154): "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 286–288): 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 14–22) 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 300–318) 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 205–223) ✓
- Co-occurrence: `sat-scope` ↔ `sat-hash` (lines 226–231) ✓
- Co-occurrence: `ceremony-id` ↔ `ceremony-type` (lines 257–262) ✓
- Co-occurrence: `merkle-proof` → `merkle-root` (lines 292–294) ✓
- 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 72–85). 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 37–137) ✓
- Unknown extension handling: verified ignored (lines 139–155) ✓
- 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 336–376) ✓
- Base64: valid and invalid merkle-proof base64 (lines 399–436) ✓
- Governance epoch: valid value, zero value (explicitly set), invalid string (lines 438–490) ✓
- Nil input: both Encode(nil) and Validate(nil) tested (lines 492–504) ✓
- UUID helper: 7 test cases including uppercase rejection (lines 522–541) ✓

**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 37–41 (SatScope struct), 46 (field type), 100–107 (Encode), 147–153 (Decode)
**Spec:** `specs/shellstream-extensions.md` lines 137–154 (Section 6.1)

The spec states at lines 152–154: "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 13–22 (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 97–112 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 286–288 (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 247–254 (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.