diff --git a/CONVENTIONS.md b/CONVENTIONS.md index ce4406a..5c3839c 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -2,8 +2,26 @@ ## Language & Dependencies -- Go standard library only — no external dependencies. - Target the latest stable Go release. +- **STRICT ALLOWLIST:** Only packages listed below may be imported. No exceptions. + +### Approved Third-Party Packages + +| Package | Use Case | Scope | +|---------|----------|-------| +| `gopkg.in/yaml.v3` | YAML parsing (persona files, config) | production | +| `github.com/google/go-cmp` | Test comparisons (`cmp.Diff`) | test only | + +**Any import not in this table or the Go standard library is forbidden.** + +Transitive dependencies of approved packages are automatically allowed. + +To request a new dependency: +1. Open a PR that ONLY updates this table +2. Requires explicit approval from Aaron +3. After merge, a separate PR may use the package + +*Enforcement: `scripts/check-deps.sh` parses this table — update only here.* ## Error Handling diff --git a/Makefile b/Makefile index 0f2e182..389bad8 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build test test-integration lint clean coverage +.PHONY: build test test-integration lint clean coverage check-deps precommit build: go build -o review-bot ./cmd/review-bot/ @@ -12,9 +12,15 @@ test-integration: lint: go vet ./... +check-deps: + @./scripts/check-deps.sh + clean: rm -f review-bot coverage: go test -coverprofile=coverage.out ./... go tool cover -func=coverage.out + +# Precommit runs all checks required before pushing +precommit: check-deps lint test diff --git a/scripts/check-deps.sh b/scripts/check-deps.sh new file mode 100755 index 0000000..f88efd2 --- /dev/null +++ b/scripts/check-deps.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# check-deps.sh - Enforces the strict dependency allowlist from CONVENTIONS.md +# Exit 1 if any unapproved import is found. +# +# Requires: Bash 4+ (for associative arrays), Go toolchain +# +# The allowlist is parsed from CONVENTIONS.md to maintain a single source of truth. +# Enforces Scope column: "test only" packages cannot appear in non-test code. + +set -euo pipefail + +# Check bash version +if ((BASH_VERSINFO[0] < 4)); then + echo "❌ Bash 4+ required (found ${BASH_VERSION})" + echo " On macOS: brew install bash" + exit 1 +fi + +CONVENTIONS_FILE="${1:-CONVENTIONS.md}" + +if [ ! -f "$CONVENTIONS_FILE" ]; then + echo "❌ CONVENTIONS.md not found" + exit 1 +fi + +# Parse approved packages from CONVENTIONS.md table using awk (POSIX-compatible) +# Format: | `package` | use case | scope | +declare -A ALLOWED_PROD=() +declare -A ALLOWED_TEST=() + +while IFS= read -r line; do + # Use awk to extract package and scope from table row + pkg=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]*`|`[[:space:]]*$/, "", $2); print $2}') + scope=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]+|[[:space:]]+$/, "", $4); print tolower($4)}') + + if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[a-zA-Z] ]]; then + if [[ "$scope" == *"test"* ]]; then + ALLOWED_TEST["$pkg"]=1 + else + ALLOWED_PROD["$pkg"]=1 + fi + fi +done < <(grep '| `' "$CONVENTIONS_FILE" 2>/dev/null || true) + +ALL_ALLOWED=("${!ALLOWED_PROD[@]}" "${!ALLOWED_TEST[@]}") + +if [ ${#ALL_ALLOWED[@]} -eq 0 ]; then + echo "⚠️ No approved packages found in $CONVENTIONS_FILE" + echo " (This is fine if you want stdlib-only)" +fi + +# Helper: check if import matches any package in an associative array (literal prefix, no glob) +matches_allowlist() { + local import="$1" + shift + local -n allowlist=$1 + + for allowed in "${!allowlist[@]}"; do + # Exact match + if [ "$import" = "$allowed" ]; then + return 0 + fi + # Literal prefix match for subpackages: must match "pkg/" exactly + if [ "${import#"$allowed/"}" != "$import" ]; then + return 0 + fi + done + return 1 +} + +# Get direct module dependencies from go.mod +DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>&1) || { + echo "❌ Failed to list dependencies: $DIRECT_IMPORTS" + exit 1 +} +DIRECT_IMPORTS=$(echo "$DIRECT_IMPORTS" | grep -v '^$' || true) + +if [ -z "$DIRECT_IMPORTS" ]; then + echo "✅ No external dependencies" + exit 0 +fi + +# Check ALL direct dependencies are in some allowlist +VIOLATIONS="" +while IFS= read -r import; do + [ -z "$import" ] && continue + + if ! matches_allowlist "$import" ALLOWED_PROD && ! matches_allowlist "$import" ALLOWED_TEST; then + VIOLATIONS="${VIOLATIONS} - ${import} (not in allowlist)"$'\n' + fi +done <<< "$DIRECT_IMPORTS" + +if [ -n "$VIOLATIONS" ]; then + echo "❌ UNAPPROVED DEPENDENCIES DETECTED" + echo "" + echo "The following imports are not in the allowlist:" + printf "%s" "$VIOLATIONS" + echo "" + echo "To add a dependency, update CONVENTIONS.md (requires Aaron's approval)" + exit 1 +fi + +# Enforce Scope: test-only packages must not appear in non-test code +# Get imports used by non-test code only (go list -deps without -test excludes test deps) +PROD_IMPORTS=$(go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' ./... 2>/dev/null || true) + +TEST_ONLY_IN_PROD="" +for test_pkg in "${!ALLOWED_TEST[@]}"; do + # Use word-boundary matching: exact match or followed by / + if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|\$|$)"; then + TEST_ONLY_IN_PROD="${TEST_ONLY_IN_PROD} - ${test_pkg} (marked 'test only' but used in production code)"$'\n' + fi +done + +if [ -n "$TEST_ONLY_IN_PROD" ]; then + echo "❌ TEST-ONLY DEPENDENCIES IN PRODUCTION CODE" + echo "" + printf "%s" "$TEST_ONLY_IN_PROD" + echo "" + echo "These packages are marked 'test only' in CONVENTIONS.md" + echo "and must only be imported from *_test.go files." + exit 1 +fi + +echo "✅ All dependencies are approved" +echo " Direct module deps: $(echo "$DIRECT_IMPORTS" | wc -l | tr -d ' ')" +echo " Production allowlist: ${#ALLOWED_PROD[@]}, Test-only allowlist: ${#ALLOWED_TEST[@]}"