diff --git a/.github/workflows/ci-go-cover.yml b/.github/workflows/ci-go-cover.yml index ac87664..eb5feae 100644 --- a/.github/workflows/ci-go-cover.yml +++ b/.github/workflows/ci-go-cover.yml @@ -12,6 +12,37 @@ jobs: uses: actions/checkout@v4 - name: Go Coverage run: | + set -euo pipefail + echo "Go version:" go version - go test -short -cover | grep -o "coverage:.*of statements$" | python scripts/cov.py + echo "Running tests with coverage..." + + # Run tests and capture coverage output with proper error handling + if ! coverage_output=$(go test -short -cover 2>&1); then + echo "Error: Go tests failed" + echo "$coverage_output" + exit 1 + fi + + echo "$coverage_output" + + # Extract coverage information with validation + if ! coverage_lines=$(echo "$coverage_output" | grep -o "coverage:.*of statements$"); then + echo "Error: No coverage information found in test output" + exit 1 + fi + + # Validate coverage script exists + if [[ ! -f "scripts/cov.py" ]]; then + echo "Error: Coverage script scripts/cov.py not found" + exit 1 + fi + + # Process coverage with error checking + if ! echo "$coverage_lines" | python scripts/cov.py; then + echo "Error: Coverage validation failed" + exit 1 + fi + + echo "Coverage validation passed!" shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 23376fc..2b59a20 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,5 +12,16 @@ jobs: uses: actions/checkout@v4 - name: Run tests run: | + set -euo pipefail + echo "Go version:" go version - go test -v + echo "Running tests..." + + # Run tests with proper error handling + if ! go test -v; then + echo "Error: Tests failed" + exit 1 + fi + + echo "All tests passed successfully!" + shell: bash diff --git a/issue-template.md b/issue-template.md new file mode 100644 index 0000000..c0ca880 --- /dev/null +++ b/issue-template.md @@ -0,0 +1,83 @@ +# Fix Critical Issues in Bash Scripts and Build System + +## Problem Summary +The current repository contains several critical issues in the Bash-related code including GitHub workflows, Makefiles, and shell commands that can lead to build failures, security vulnerabilities, and maintenance problems. + +## Issues Identified + +### 1. ๐Ÿšจ **Critical: Unsafe Pipeline Construction in Coverage Workflow** +**File**: `.github/workflows/ci-go-cover.yml` +**Issue**: The coverage pipeline uses potentially unsafe command chaining without proper error handling: +```bash +go test -short -cover | grep -o "coverage:.*of statements$" | python scripts/cov.py +``` +**Risk**: Silent failures in the pipeline where `grep` or `python` could fail without proper error detection. + +### 2. โš ๏ธ **Build System Fragility** +**File**: `utils/Makefile` +**Issue**: Hard failure when `zek` dependency is missing with poor error recovery: +```makefile +zek ?= $(shell command -v zek) +ifeq ($(strip $(zek)),) +$(error zek not found. To install zek: 'go install github.com/miku/zek/cmd/zek@latest') +endif +``` +**Risk**: Breaks the entire build process instead of providing graceful degradation or auto-installation. + +### 3. ๐Ÿ”’ **Security: Missing Error Handling in GitHub Workflows** +**Files**: `.github/workflows/ci-go-cover.yml`, `.github/workflows/ci.yml` +**Issue**: No `set -euo pipefail` or equivalent error handling in shell scripts. +**Risk**: Commands may fail silently, leading to false positive test results. + +### 4. ๐Ÿ“ฆ **Dependency Management Issues** +**File**: `utils/Makefile` +**Issue**: External dependency (`zek`) is required but not automatically managed. +**Risk**: New contributors face immediate build failures without clear resolution paths. + +### 5. ๐Ÿงช **Missing Test Coverage for Build Scripts** +**Issue**: No validation or testing of the Makefile targets and GitHub workflow scripts. +**Risk**: Build system regressions go unnoticed until they cause production issues. + +## Proposed Solutions + +### 1. **Enhanced GitHub Workflows** +- Add proper error handling with `set -euo pipefail` +- Implement proper exit status checking for pipeline commands +- Add timeout mechanisms for long-running processes +- Separate concerns for better debugging + +### 2. **Improved Makefile Robustness** +- Add auto-installation targets for missing dependencies +- Implement graceful degradation when optional tools are missing +- Add validation targets to check system requirements +- Better error messages with actionable instructions + +### 3. **Security Enhancements** +- Validate all input parameters +- Use safer shell scripting practices +- Add explicit error handling for all external commands +- Implement proper cleanup mechanisms + +### 4. **Build System Testing** +- Add validation tests for Makefile targets +- Create integration tests for GitHub workflows +- Implement pre-commit hooks to validate script changes + +## Impact Assessment +- **Severity**: High - Affects build reliability and security +- **Scope**: All contributors and CI/CD processes +- **Urgency**: High - Should be fixed before next release + +## Acceptance Criteria +- [ ] All shell scripts use proper error handling (`set -euo pipefail`) +- [ ] GitHub workflows have explicit error checking for all commands +- [ ] Makefiles provide graceful handling of missing dependencies +- [ ] Build system includes validation and testing +- [ ] Documentation updated with troubleshooting guides +- [ ] All existing tests pass with new implementations + +## Additional Context +This issue was identified during a comprehensive audit of the repository's build system. The changes will improve reliability for all contributors and reduce the likelihood of silent failures in CI/CD processes. + +## Labels +`bug`, `enhancement`, `CI/CD`, `build-system`, `high-priority` \ No newline at end of file diff --git a/scripts/validate-build.sh b/scripts/validate-build.sh new file mode 100755 index 0000000..bf93abf --- /dev/null +++ b/scripts/validate-build.sh @@ -0,0 +1,208 @@ +#!/bin/bash + +# Build system validation script for CMW project +# This script validates that all build dependencies and processes work correctly + +set -euo pipefail + +# Color codes for output +readonly RED='\033[0;31m' +readonly GREEN='\033[0;32m' +readonly YELLOW='\033[1;33m' +readonly NC='\033[0m' # No Color + +# Script configuration +readonly SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +readonly PROJECT_ROOT="$(dirname "$SCRIPT_DIR")" + +# Logging functions +log_info() { + echo -e "${GREEN}[INFO]${NC} $*" +} + +log_warn() { + echo -e "${YELLOW}[WARN]${NC} $*" +} + +log_error() { + echo -e "${RED}[ERROR]${NC} $*" >&2 +} + +# Validation functions +check_go_installation() { + log_info "Checking Go installation..." + if ! command -v go >/dev/null 2>&1; then + log_error "Go is not installed or not in PATH" + return 1 + fi + + local go_version + go_version=$(go version) + log_info "Found: $go_version" + return 0 +} + +check_system_dependencies() { + log_info "Checking system dependencies..." + + local missing_deps=() + + # Check for curl + if ! command -v curl >/dev/null 2>&1; then + missing_deps+=("curl") + fi + + # Check for make + if ! command -v make >/dev/null 2>&1; then + missing_deps+=("make") + fi + + if [[ ${#missing_deps[@]} -gt 0 ]]; then + log_error "Missing system dependencies: ${missing_deps[*]}" + return 1 + fi + + log_info "All system dependencies found" + return 0 +} + +validate_utils_makefile() { + log_info "Validating utils Makefile..." + + cd "$PROJECT_ROOT/utils" || { + log_error "Cannot access utils directory" + return 1 + } + + # Check if we can install dependencies + if ! make install-deps; then + log_error "Failed to install utils dependencies" + return 1 + fi + + # Validate the build environment + if ! make validate; then + log_error "Utils build environment validation failed" + return 1 + fi + + # Test dry-run build + if ! make --dry-run all; then + log_error "Utils Makefile dry-run failed" + return 1 + fi + + log_info "Utils Makefile validation passed" + return 0 +} + +validate_testdata_makefile() { + log_info "Validating testdata Makefile..." + + cd "$PROJECT_ROOT/testdata" || { + log_error "Cannot access testdata directory" + return 1 + } + + # Validate the build environment + if make validate; then + log_info "Testdata validation passed" + + # Test dry-run build if dependencies are available + if make --dry-run all; then + log_info "Testdata Makefile dry-run passed" + else + log_warn "Testdata Makefile dry-run failed (dependencies may be missing)" + fi + else + log_warn "Testdata validation failed (missing diag2cbor.rb)" + log_info "This is acceptable if CBOR tools are not installed" + fi + + return 0 +} + +validate_go_tests() { + log_info "Validating Go tests..." + + cd "$PROJECT_ROOT" || { + log_error "Cannot access project root" + return 1 + } + + # Check if tests compile + if ! go test -c -o /dev/null ./...; then + log_error "Go tests do not compile" + return 1 + fi + + # Run short tests + if ! go test -short ./...; then + log_error "Go short tests failed" + return 1 + fi + + log_info "Go tests validation passed" + return 0 +} + +validate_workflows() { + log_info "Validating GitHub workflows..." + + local workflow_dir="$PROJECT_ROOT/.github/workflows" + + if [[ ! -d "$workflow_dir" ]]; then + log_error "GitHub workflows directory not found" + return 1 + fi + + # Check workflow files exist and are readable + local workflows=("ci.yml" "ci-go-cover.yml") + for workflow in "${workflows[@]}"; do + local workflow_path="$workflow_dir/$workflow" + if [[ ! -f "$workflow_path" ]]; then + log_error "Workflow file not found: $workflow" + return 1 + fi + + if [[ ! -r "$workflow_path" ]]; then + log_error "Workflow file not readable: $workflow" + return 1 + fi + + log_info "Found workflow: $workflow" + done + + log_info "GitHub workflows validation passed" + return 0 +} + +# Main validation function +main() { + log_info "Starting CMW build system validation..." + + local validation_errors=0 + + # Run all validations + check_go_installation || ((validation_errors++)) + check_system_dependencies || ((validation_errors++)) + validate_go_tests || ((validation_errors++)) + validate_workflows || ((validation_errors++)) + validate_utils_makefile || ((validation_errors++)) + validate_testdata_makefile || ((validation_errors++)) + + # Report results + echo + if [[ $validation_errors -eq 0 ]]; then + log_info "โœ… All validations passed! Build system is healthy." + return 0 + else + log_error "โŒ $validation_errors validation(s) failed. Please address the issues above." + return 1 + fi +} + +# Script entry point +if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then + main "$@" +fi \ No newline at end of file diff --git a/testdata/Makefile b/testdata/Makefile index b538de0..3e981a4 100644 --- a/testdata/Makefile +++ b/testdata/Makefile @@ -4,10 +4,44 @@ DIAG_TESTDATA := $(wildcard *.diag) CBOR_TESTDATA := $(DIAG_TESTDATA:.diag=.cbor) -%.cbor: %.diag ; diag2cbor.rb $< > $@ +.PHONY: all clean check-deps validate -CLEANFILES += $(CBOR_TESTDATA) +# Default target with dependency checking +all: check-deps $(CBOR_TESTDATA) + +# Check for required dependencies +check-deps: + @echo "Checking dependencies..." + @if ! command -v diag2cbor.rb >/dev/null 2>&1; then \ + echo "Error: diag2cbor.rb not found in PATH"; \ + echo "Please install the CBOR diagnostic tools."; \ + exit 1; \ + fi + @echo "All dependencies found." + +# Validate the build environment +validate: check-deps + @echo "Validating testdata build environment..." + @if [ -z "$(DIAG_TESTDATA)" ]; then \ + echo "Warning: No .diag files found to process."; \ + else \ + echo "Found $(words $(DIAG_TESTDATA)) .diag files to process."; \ + fi + @echo "Build environment validated." -all: $(CBOR_TESTDATA) +# Convert .diag files to .cbor with error checking +%.cbor: %.diag + @echo "Converting $< to $@..." + @if ! diag2cbor.rb $< > $@; then \ + echo "Error: Failed to convert $< to $@"; \ + rm -f $@; \ + exit 1; \ + fi + @echo "Successfully converted $< to $@" + +CLEANFILES += $(CBOR_TESTDATA) -clean: ; $(RM) -f $(CLEANFILES) +clean: + @echo "Cleaning generated CBOR files..." + @$(RM) -f $(CLEANFILES) + @echo "Clean completed." diff --git a/utils/Makefile b/utils/Makefile index 3f5e4f7..ae499a2 100644 --- a/utils/Makefile +++ b/utils/Makefile @@ -7,26 +7,83 @@ REGISTRY_GO := core-parameters.go EXE := main CLEANFILES := -zek ?= $(shell command -v zek) -ifeq ($(strip $(zek)),) -$(error zek not found. To install zek: 'go install github.com/miku/zek/cmd/zek@latest') -endif +# Check for zek availability with better error handling +zek ?= $(shell command -v zek 2>/dev/null) -all: $(MAP_FILE) +.PHONY: all clean check-deps install-deps validate + +# Default target with dependency checking +all: check-deps $(MAP_FILE) + +# Check system dependencies +check-deps: + @echo "Checking dependencies..." + @if [ -z "$(zek)" ]; then \ + echo "Warning: zek not found. Run 'make install-deps' to install it."; \ + echo "Or install manually: go install github.com/miku/zek/cmd/zek@latest"; \ + exit 1; \ + fi + @echo "All dependencies found." + +# Install missing dependencies +install-deps: + @echo "Installing zek dependency..." + @if ! command -v go >/dev/null 2>&1; then \ + echo "Error: Go is required but not installed."; \ + exit 1; \ + fi + @go install github.com/miku/zek/cmd/zek@latest + @echo "Dependencies installed successfully." + +# Validate the build environment +validate: check-deps + @echo "Validating build environment..." + @if [ ! -f "$(REGISTRY)" ] && ! curl --version >/dev/null 2>&1; then \ + echo "Error: curl is required to download registry but not found."; \ + exit 1; \ + fi + @echo "Build environment validated." $(MAP_FILE): $(REGISTRY) $(EXE) - ./$(EXE) < $(REGISTRY) | gofmt > $@ + @echo "Generating content format map..." + @if ! ./$(EXE) < $(REGISTRY) | gofmt > $@; then \ + echo "Error: Failed to generate $(MAP_FILE)"; \ + rm -f $@; \ + exit 1; \ + fi + @echo "Generated $(MAP_FILE) successfully." -$(EXE): $(REGISTRY_GO) ; go build +$(EXE): $(REGISTRY_GO) + @echo "Building $(EXE)..." + @if ! go build -o $(EXE); then \ + echo "Error: Failed to build $(EXE)"; \ + exit 1; \ + fi CLEANFILES += $(EXE) -$(REGISTRY_GO): $(REGISTRY) ; cat $< | $(zek) -P main > $@ +$(REGISTRY_GO): $(REGISTRY) + @echo "Generating Go code from registry..." + @if ! cat $< | $(zek) -P main > $@; then \ + echo "Error: Failed to generate $(REGISTRY_GO)"; \ + rm -f $@; \ + exit 1; \ + fi CLEANFILES += $(REGISTRY_GO) -$(REGISTRY): ; curl -sO $(REGISTRY_URL) +$(REGISTRY): + @echo "Downloading registry..." + @if ! curl -fsSL -o $@ $(REGISTRY_URL); then \ + echo "Error: Failed to download $(REGISTRY)"; \ + rm -f $@; \ + exit 1; \ + fi + @echo "Downloaded $(REGISTRY) successfully." CLEANFILES += $(REGISTRY) -clean: ; rm -f $(CLEANFILES) \ No newline at end of file +clean: + @echo "Cleaning build artifacts..." + @rm -f $(CLEANFILES) + @echo "Clean completed." \ No newline at end of file