diff --git a/.gitignore b/.gitignore index ff0c30ce..e82669e0 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,10 @@ bootstrap-linux-arm64.sum.txt # For SKD debugging while building images spike-sdk-go +# SPIRE source clone (build-spire.sh now uses a temp dir; guard against a +# repo-root clone from older runs or a manual `git clone`). +/spire/ + # WSL *:Zone.Identifier diff --git a/app/nexus/internal/route/acl/policy/delete_intercept.go b/app/nexus/internal/route/acl/policy/delete_intercept.go index c77a5f20..d306c23a 100644 --- a/app/nexus/internal/route/acl/policy/delete_intercept.go +++ b/app/nexus/internal/route/acl/policy/delete_intercept.go @@ -20,21 +20,21 @@ import ( // // The function performs the following validations in order: // - Extracts and validates the peer SPIFFE ID from the request -// - Validates the policy ID format +// - Validates the policy name format // - Checks if the peer has write permission for the policy access path // // If any validation fails, an appropriate error response is written to the // ResponseWriter and an error is returned. // // Parameters: -// - request: The policy deletion request containing the policy ID +// - request: The policy deletion request containing the policy name // - w: The HTTP response writer for error responses // - r: The HTTP request containing the peer SPIFFE ID // // Returns: // - *sdkErrors.SDKError: nil if all validations pass, // ErrAccessUnauthorized if authentication or authorization fails, -// ErrDataInvalidInput if policy ID validation fails +// ErrDataInvalidInput if policy name validation fails func guardPolicyDeleteRequest( request reqres.PolicyDeleteRequest, w http.ResponseWriter, r *http.Request, ) *sdkErrors.SDKError { @@ -47,7 +47,11 @@ func guardPolicyDeleteRequest( return authErr } - return net.RespondErrOnBadPolicyID( - request.ID, w, reqres.PolicyDeleteResponse{}.BadRequest(), + // SPIKE policies are keyed by name; the SDK's PolicyDeleteRequest + // still exposes that identifier as ID, so request.ID holds the policy + // name and must be validated as a name, not as a UUID. See issue #250 + // for the pending SDK field rename. + return net.RespondErrOnBadName( + request.ID, reqres.PolicyDeleteResponse{}.BadRequest(), w, ) } diff --git a/app/nexus/internal/route/acl/policy/get_intercept.go b/app/nexus/internal/route/acl/policy/get_intercept.go index 1e958389..88ae59e7 100644 --- a/app/nexus/internal/route/acl/policy/get_intercept.go +++ b/app/nexus/internal/route/acl/policy/get_intercept.go @@ -20,21 +20,21 @@ import ( // // The function performs the following validations in order: // - Extracts and validates the peer SPIFFE ID from the request -// - Validates the policy ID format +// - Validates the policy name format // - Checks if the peer has read permission for the policy access path // // If any validation fails, an appropriate error response is written to the // ResponseWriter and an error is returned. // // Parameters: -// - request: The policy read request containing the policy ID +// - request: The policy read request containing the policy name // - w: The HTTP response writer for error responses // - r: The HTTP request containing the peer SPIFFE ID // // Returns: // - *sdkErrors.SDKError: nil if all validations pass, // ErrAccessUnauthorized if authentication or authorization fails, -// ErrDataInvalidInput if policy ID validation fails +// ErrDataInvalidInput if policy name validation fails func guardPolicyReadRequest( request reqres.PolicyReadRequest, w http.ResponseWriter, r *http.Request, ) *sdkErrors.SDKError { @@ -47,7 +47,11 @@ func guardPolicyReadRequest( return authErr } - return net.RespondErrOnBadPolicyID( - request.ID, w, reqres.PolicyReadResponse{}.BadRequest(), + // SPIKE policies are keyed by name; the SDK's PolicyReadRequest still + // exposes that identifier as ID, so request.ID holds the policy name + // and must be validated as a name, not as a UUID. See issue #250 for + // the pending SDK field rename. + return net.RespondErrOnBadName( + request.ID, reqres.PolicyReadResponse{}.BadRequest(), w, ) } diff --git a/app/spike/internal/cmd/policy/format.go b/app/spike/internal/cmd/policy/format.go index 4b036163..ff6ef4d5 100644 --- a/app/spike/internal/cmd/policy/format.go +++ b/app/spike/internal/cmd/policy/format.go @@ -19,8 +19,9 @@ import ( // formatPoliciesOutput formats the output of policy list items based on the // format flag. It supports human/plain, json, and yaml formats. For human -// format, it creates a readable tabular representation. For JSON/YAML formats, -// it marshals the policies to the appropriate structured format. +// format, it creates a readable list of policy names (policies are keyed by +// name; there is no separate ID). For JSON/YAML formats, it marshals the +// policies to the appropriate structured format. // // If the format flag is invalid, it returns an error message. // If the "policies" list is empty, it returns an appropriate message based on @@ -28,7 +29,7 @@ import ( // // Parameters: // - cmd: The Cobra command containing the format flag -// - policies: The policy list items to format (contains ID and Name only) +// - policies: The policy list items to format // // Returns: // - string: The formatted output or error message @@ -73,7 +74,6 @@ func formatPoliciesOutput( result.WriteString("POLICIES\n========\n\n") for _, policy := range *policies { - result.WriteString(fmt.Sprintf("ID: %s\n", policy.ID)) result.WriteString(fmt.Sprintf("Name: %s\n", policy.Name)) result.WriteString("--------\n\n") } @@ -120,7 +120,6 @@ func formatPolicy(cmd *cobra.Command, policy *data.Policy) string { var result strings.Builder result.WriteString("POLICY DETAILS\n=============\n\n") - result.WriteString(fmt.Sprintf("ID: %s\n", policy.ID)) result.WriteString(fmt.Sprintf("Name: %s\n", policy.Name)) result.WriteString(fmt.Sprintf("SPIFFE ID Pattern: %s\n", policy.SPIFFEIDPattern)) diff --git a/app/spike/internal/cmd/policy/format_test.go b/app/spike/internal/cmd/policy/format_test.go index 8ddad65c..b718d00f 100644 --- a/app/spike/internal/cmd/policy/format_test.go +++ b/app/spike/internal/cmd/policy/format_test.go @@ -104,9 +104,9 @@ func TestFormatPoliciesOutput_HumanFormat(t *testing.T) { t.Error("Human format should contain 'POLICIES' header") } - // Check policy fields are present (PolicyListItem only has ID and Name) + // The human format prints only the name; policies are keyed by name + // and the vestigial ID is not shown. expectedFields := []string{ - "ID: 123e4567-e89b-12d3-a456-426614174000", "Name: test-policy", } @@ -200,7 +200,6 @@ func TestFormatPolicy_HumanFormat(t *testing.T) { // Check all fields are present expectedFields := []string{ - "ID: 123e4567-e89b-12d3-a456-426614174000", "Name: admin-policy", "SPIFFE ID Pattern: ^spiffe://example\\.org/admin/.*$", "Path Pattern: ^.*$", diff --git a/app/spike/internal/cmd/policy/list.go b/app/spike/internal/cmd/policy/list.go index a3097696..b362881d 100644 --- a/app/spike/internal/cmd/policy/list.go +++ b/app/spike/internal/cmd/policy/list.go @@ -51,31 +51,23 @@ import ( // spike policy list --path-pattern="^secrets/db/.*$" // spike policy list --spiffeid-pattern="^spiffe://example\.org/app$" // -// Example output for human format: +// Example output for human format (the list shows policy names only; use +// `spike policy get` for the full details of a policy): // // POLICIES // ======== // -// ID: policy-123 // Name: web-service-policy -// SPIFFE ID Pattern: ^spiffe://example\.org/web-service/.*$ -// Path Pattern: ^secrets/db/.*$ -// Permissions: read, write -// Created At: 2024-01-01T00:00:00Z -// Created By: user-abc // -------- // -// Example output for JSON format: +// Example output for JSON format (the "id" field is always empty; policies +// are keyed by name, and the field awaits the SDK rename tracked in issue +// #250): // // [ // { -// "id": "policy-123", -// "name": "web-service-policy", -// "spiffeIdPattern": "^spiffe://example\.org/web-service/.*$", -// "pathPattern": "^tenants/demo/db$", -// "permissions": ["read", "write"], -// "createdAt": "2024-01-01T00:00:00Z", -// "createdBy": "user-abc" +// "id": "", +// "name": "web-service-policy" // } // ] // diff --git a/hack/bare-metal/build/build-spire.sh b/hack/bare-metal/build/build-spire.sh index ad977654..86c25658 100755 --- a/hack/bare-metal/build/build-spire.sh +++ b/hack/bare-metal/build/build-spire.sh @@ -5,8 +5,27 @@ # \\\\\\\ SPDX-License-Identifier: Apache-2.0 # This is a simple way to create a single-node SPIRE development setup. -git clone --single-branch --branch v1.11.2 https://github.com/spiffe/spire.git -cd spire || exit +# It builds the SPIRE server and agent from source and installs them into +# /usr/local/bin. The source is cloned into a temporary directory that is +# removed on exit, so the repository working tree stays clean. + +set -euo pipefail + +SPIRE_VERSION="v1.11.2" + +# Clone into a temporary directory and always clean it up on exit, even on +# failure. Leaving the clone behind would pollute the working tree and break +# `make audit` (gofmt scans it). +CLONE_DIR="$(mktemp -d)" +cleanup() { + rm -rf "$CLONE_DIR" +} +trap cleanup EXIT + +git clone --single-branch --branch "$SPIRE_VERSION" \ + https://github.com/spiffe/spire.git "$CLONE_DIR" + +cd "$CLONE_DIR" go build ./cmd/spire-server go build ./cmd/spire-agent sudo mv spire-server /usr/local/bin diff --git a/hack/bare-metal/startup/start.sh b/hack/bare-metal/startup/start.sh index 01a30455..efdd5194 100755 --- a/hack/bare-metal/startup/start.sh +++ b/hack/bare-metal/startup/start.sh @@ -79,6 +79,63 @@ fi # Your existing script continues here echo "Domain check passed. Continuing with the script..." +# Preflight: make sure the SPIRE binaries this run needs are on PATH. +# Failing here with clear guidance is far more helpful than a bare +# "spire-server: command not found" surfacing later from the agent-token +# or agent-start scripts. Only the binaries that the enabled steps actually +# use are required, so external-SPIRE workflows that skip those steps are +# not blocked. +check_spire_binaries() { + local missing=() + + # spire-server starts the server, generates the agent token, and + # registers entries. + if [ -z "$SPIKE_SKIP_SPIRE_SERVER_START" ] || + [ -z "$SPIKE_SKIP_GENERATE_AGENT_TOKEN" ] || + [ -z "$SPIKE_SKIP_REGISTER_ENTRIES" ]; then + if ! command -v spire-server >/dev/null 2>&1; then + missing+=("spire-server") + fi + fi + + # spire-agent starts the SPIRE agent. + if [ -z "$SPIKE_SKIP_SPIRE_AGENT_START" ]; then + if ! command -v spire-agent >/dev/null 2>&1; then + missing+=("spire-agent") + fi + fi + + if [ ${#missing[@]} -eq 0 ]; then + return 0 + fi + + echo "" >&2 + echo "Error: required SPIRE binaries were not found on your PATH:" >&2 + for m in "${missing[@]}"; do + echo " - $m" >&2 + done + echo "" >&2 + echo "The bare-metal dev environment needs the SPIRE server and agent" >&2 + echo "binaries on your PATH." >&2 + echo "" >&2 + echo "To build and install them (SPIRE v1.11.2 into /usr/local/bin):" >&2 + echo " make build-spire" >&2 + echo "" >&2 + echo "If you already have them elsewhere, add that directory to PATH:" >&2 + echo " export PATH=\"/path/to/spire/bin:\$PATH\"" >&2 + echo "" >&2 + echo "Then verify with:" >&2 + echo " command -v spire-server spire-agent" >&2 + echo "" >&2 + echo "See https://spike.ist/development/bare-metal/ for details." >&2 + return 1 +} + +if ! check_spire_binaries; then + echo "SPIRE preflight check failed. Exiting..." >&2 + exit 1 +fi + # Helpers source ./hack/lib/bg.sh @@ -102,32 +159,97 @@ if [ -z "$SPIKE_SKIP_SPIKE_BUILD" ]; then fi else echo "SPIKE_SKIP_SPIKE_BUILD is set, skipping SPIKE build." +fi - # Only check for binaries if we're skipping the build step - echo "Checking for required SPIKE binaries..." - REQUIRED_BINARIES=("spike" "nexus" "keeper" "bootstrap") - MISSING_BINARIES=() - - for binary in "${REQUIRED_BINARIES[@]}"; do - if ! command -v "$binary" >/dev/null 2>&1; then - MISSING_BINARIES+=("$binary") +# Preflight: make sure the SPIKE binaries are on PATH. +# +# build-spike.sh compiles them into ./bin, but this script and its child +# scripts invoke them as bare commands (spike, nexus, keeper, bootstrap, +# demo), so the bin directory must also be on PATH. The check runs after +# the build step because on a fresh clone the binaries do not exist until +# the build produces them. Failing here, before any SPIRE or SPIKE +# process starts, is far more helpful than a bare "spike: command not +# found" surfacing halfway through startup. +# +# Each name must also resolve to the binary in ./bin, not to an +# unrelated tool that happens to share the name. The SPIRE entries pin +# the exact binary path and hash (unix:path and unix:sha256 selectors), +# so a lookalike elsewhere on PATH cannot obtain an identity and the +# run fails in confusing ways later. +check_spike_binaries() { + local missing=() + local shadowed=() + local found + local failed=0 + + for b in spike nexus keeper bootstrap demo; do + found="$(command -v "$b" 2>/dev/null)" + if [ -z "$found" ]; then + missing+=("$b") + elif [ ! "$found" -ef "$(pwd)/bin/$b" ]; then + shadowed+=("$b -> $found") fi done - if [ ${#MISSING_BINARIES[@]} -gt 0 ]; then - echo "Error: The following required binaries are not found in PATH:" - for missing in "${MISSING_BINARIES[@]}"; do - echo " - $missing" + if [ ${#missing[@]} -gt 0 ]; then + failed=1 + echo "" >&2 + echo "Error: required SPIKE binaries were not found on your PATH:" >&2 + for m in "${missing[@]}"; do + echo " - $m" >&2 done - echo "" - echo "Please build SPIKE binaries first by running:" - echo " ./hack/bare-metal/build/build-spike.sh" - echo "" - echo "Or unset SPIKE_SKIP_SPIKE_BUILD to build them automatically." - exit 1 + echo "" >&2 + echo "To build them into ./bin:" >&2 + echo " make build" >&2 + echo "" >&2 + echo "Then add the bin directory to your PATH:" >&2 + echo " export PATH=\"\$PATH:$(pwd)/bin\"" >&2 + echo "" >&2 + echo "Then verify with:" >&2 + echo " command -v spike nexus keeper bootstrap demo" >&2 + echo "" >&2 + echo "See https://spike.ist/development/bare-metal/ for details." >&2 + fi + + if [ ${#shadowed[@]} -gt 0 ]; then + local label="Error" + if [ -n "$SPIKE_SKIP_REGISTER_ENTRIES" ]; then + label="Warning" + fi + echo "" >&2 + echo "$label: PATH resolves these commands outside $(pwd)/bin:" >&2 + for s in "${shadowed[@]}"; do + echo " - $s" >&2 + done + echo "" >&2 + echo "SPIKE registers its workloads with SPIRE by exact binary" >&2 + echo "path and hash, so the binaries above cannot obtain an" >&2 + echo "identity even if they are copies of the SPIKE binaries." >&2 + echo "" >&2 + echo "Give $(pwd)/bin precedence for this shell:" >&2 + echo " export PATH=\"$(pwd)/bin:\$PATH\"" >&2 + echo "" >&2 + echo "Or remove/rename the conflicting binaries." >&2 + if [ -z "$SPIKE_SKIP_REGISTER_ENTRIES" ]; then + failed=1 + else + echo "" >&2 + echo "WARNING: continuing because SPIKE_SKIP_REGISTER_ENTRIES" >&2 + echo "is set; make sure your registered entries match the" >&2 + echo "binaries listed above." >&2 + fi fi - echo "All required SPIKE binaries found." + if [ $failed -eq 0 ] && [ ${#shadowed[@]} -eq 0 ]; then + echo "All required SPIKE binaries found on PATH." + fi + + return $failed +} + +if ! check_spike_binaries; then + echo "SPIKE binary preflight check failed. Exiting..." >&2 + exit 1 fi # Start SPIRE server in background and save its PID @@ -298,26 +420,55 @@ if [ $POLICY_EXIT_CODE -ne 0 ]; then POLICY_VALIDATION_FAILED=true fi -# Validate expected policy output (warnings only, no exit) +# Validate expected policy output (warnings only, no exit). +# `spike policy list` prints one "Name: " block per policy; +# policies are keyed by name, and the list shows no other fields. echo "$POLICY_OUTPUT" | grep -q "POLICIES" || \ { echo "WARNING: Missing 'POLICIES' header in output"; POLICY_VALIDATION_FAILED=true; } -echo "$POLICY_OUTPUT" | grep -qE '^ID[[:space:]]+NAME$' || \ - { echo "WARNING: Missing 'ID NAME' header in output"; POLICY_VALIDATION_FAILED=true; } -echo "$POLICY_OUTPUT" | grep -q "workload-can-read" || \ +echo "$POLICY_OUTPUT" | grep -q "Name: workload-can-read" || \ { echo "WARNING: Missing 'workload-can-read' policy"; POLICY_VALIDATION_FAILED=true; } -echo "$POLICY_OUTPUT" | grep -q "workload-can-write" || \ +echo "$POLICY_OUTPUT" | grep -q "Name: workload-can-write" || \ { echo "WARNING: Missing 'workload-can-write' policy"; POLICY_VALIDATION_FAILED=true; } -echo "$POLICY_OUTPUT" | grep -q "Permissions: read" || \ + +# Permissions are only visible via `spike policy get`; the demo policies +# carry one permission each (read and write, respectively). This also +# exercises the get-by-name path end to end. +POLICY_READ_OUTPUT=$(spike policy get workload-can-read 2>&1) +POLICY_READ_EXIT_CODE=$? + +if [ $POLICY_READ_EXIT_CODE -ne 0 ]; then + echo "WARNING: Policy get workload-can-read failed" \ + "with exit code $POLICY_READ_EXIT_CODE" + echo "Output:" + echo "$POLICY_READ_OUTPUT" + POLICY_VALIDATION_FAILED=true +fi +echo "$POLICY_READ_OUTPUT" | grep -q "Permissions: read" || \ { echo "WARNING: Missing read permission"; POLICY_VALIDATION_FAILED=true; } -echo "$POLICY_OUTPUT" | grep -q "Permissions: write" || \ + +POLICY_WRITE_OUTPUT=$(spike policy get workload-can-write 2>&1) +POLICY_WRITE_EXIT_CODE=$? + +if [ $POLICY_WRITE_EXIT_CODE -ne 0 ]; then + echo "WARNING: Policy get workload-can-write failed" \ + "with exit code $POLICY_WRITE_EXIT_CODE" + echo "Output:" + echo "$POLICY_WRITE_OUTPUT" + POLICY_VALIDATION_FAILED=true +fi +echo "$POLICY_WRITE_OUTPUT" | grep -q "Permissions: write" || \ { echo "WARNING: Missing write permission"; POLICY_VALIDATION_FAILED=true; } if [ "$POLICY_VALIDATION_FAILED" = true ]; then echo "" echo "==========================================" echo "POLICY VALIDATION FAILED (debug mode - continuing anyway)" - echo "Full policy output:" + echo "Full policy list output:" echo "$POLICY_OUTPUT" + echo "Full policy get output (workload-can-read):" + echo "$POLICY_READ_OUTPUT" + echo "Full policy get output (workload-can-write):" + echo "$POLICY_WRITE_OUTPUT" echo "==========================================" else echo "Policy verification passed." @@ -416,7 +567,7 @@ echo "> Everything is set up." echo "> You can now experiment with SPIKE." echo ">" echo "<<" -echo "> >> To begin, run './spike' on a separate terminal window." +echo "> >> To begin, run 'spike' on a separate terminal window." echo "<<" echo ">" echo "> When you are done with your experiments, you can press 'Ctrl+C'" diff --git a/makefiles/BareMetal.mk b/makefiles/BareMetal.mk index 66ca144d..5a40ab82 100644 --- a/makefiles/BareMetal.mk +++ b/makefiles/BareMetal.mk @@ -30,6 +30,14 @@ start-privileged: build: ./hack/bare-metal/build/build-spike.sh +.PHONY: build-spire +# Builds and installs the SPIRE server and agent binaries that the +# bare-metal dev environment needs (SPIRE v1.11.2 into /usr/local/bin). +# Requires Go and sudo. Run this once before `make start` if +# spire-server/spire-agent are not already on your PATH. +build-spire: + ./hack/bare-metal/build/build-spire.sh + # Registry an entry to the SPIRE server for the demo app. demo-register-entry: ./examples/consume-secrets/demo-register-entry.sh