diff --git a/create-project.sh b/create-project.sh index 1670d4a..1744752 100644 --- a/create-project.sh +++ b/create-project.sh @@ -29,11 +29,27 @@ # # Requires # bash 4.4 or later, git, curl, mktemp; jq is optional (used when present). +# Also the base tools sed, grep, head, tr, rm, rmdir and uname, and stat +# (GNU "stat -c" or BSD "stat -f"; only used outside Windows). +# +# Implements +# MIL-001 tasks 1 to 6 (issues #3 to #8), user story US-001.01 and UC-001 +# steps 1 to 3; see docs/. Deviation from the request: its second +# GITEA_URL key is named GITEA_API_URL. +# +# Tracing +# set -x is switched off while the script runs, because a trace would print +# every secret the script handles. # # Exit codes # 0 success, 1 a failed check or bad input, 2 a usage error. set -Eeuo pipefail +if [[ $- == *x* ]]; then + set +x + printf 'warning: tracing (set -x) is disabled because it would print secrets\n' >&2 +fi + if ((BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 4))); then printf 'error: bash 4.4 or later is required (found %s)\n' "$BASH_VERSION" >&2 exit 1 @@ -258,6 +274,9 @@ parse_env_file() { # shellcheck disable=SC2094 # the loop body only uses $file in messages while IFS= read -r line || [[ -n $line ]]; do line_number=$((line_number + 1)) + if ((line_number == 1)); then + line="${line#$'\xEF\xBB\xBF'}" # byte order mark from some Windows editors + fi line="$(trim "${line%$'\r'}")" if [[ -z $line || $line == \#* ]]; then continue @@ -588,9 +607,9 @@ collect_project_details() { "use letters, digits, '.', '_' or '-' (at most 39)" PROJECT[gitea_owner]="$REPLY" prompt_yes_no "Also create a GitHub repository (applies the AGPL license)" y - PROJECT[use_github]="$REPLY" + PROJECT[has_github]="$REPLY" PROJECT[github_owner]="" - if ((PROJECT[use_github])); then + if ((PROJECT[has_github])); then prompt_value "GitHub owner (user or organization)" \ "${CREDENTIALS[GITHUB_USER]:-}" is_valid_github_owner \ "use letters, digits or '-' (at most 39)" @@ -600,7 +619,7 @@ collect_project_details() { "must not be empty, start with '-' or contain control characters" PROJECT[directory]="$REPLY" prompt_yes_no "Enable the plan gate" n - PROJECT[plan_gate]="$REPLY" + PROJECT[is_plan_gate_enabled]="$REPLY" } # ------------------------------------------------------------- summary @@ -628,13 +647,13 @@ print_summary() { say " Repository : ${PROJECT[name]} (${PROJECT[visibility]})" say " Description : ${PROJECT[description]:-(none)}" say " Gitea : ${CONFIG[GITEA_URL]}/${PROJECT[gitea_owner]}/${PROJECT[name]}" - if ((PROJECT[use_github])); then + if ((PROJECT[has_github])); then say " GitHub : ${CONFIG[GITHUB_WEB_URL]}/${PROJECT[github_owner]}/${PROJECT[name]} (AGPL license applied)" else say " GitHub : not used" fi say " Directory : ${PROJECT[directory]}" - say " Plan gate : $(yes_no "${PROJECT[plan_gate]}")" + say " Plan gate : $(yes_no "${PROJECT[is_plan_gate_enabled]}")" say "Credentials : GITEA_TOKEN $(credential_state GITEA_TOKEN)," \ "GITHUB_PAT $(credential_state GITHUB_PAT)" say "Creating the repositories and the project comes in later phases." @@ -678,7 +697,7 @@ main() { setup_temp_dir load_configuration collect_project_details - if ((PROJECT[use_github])); then + if ((PROJECT[has_github])); then require_github_credentials fi print_summary diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index 50f6de2..3f43a87 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -23,7 +23,7 @@ document of a type. `Primary File` may contain a glob (e.g. | DM | Domain Model | docs/domain-model.md | 003 | | DICT | Domain Dictionary (PO and IT terms) | docs/dictionary.md | 002 | | UCD | Use Case Diagram | docs/use-case-diagram.md | 002 | -| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 016 | +| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 017 | | TM | Traceability Matrix | docs/sqa/traceability-matrix.md | 002 | ## Languages diff --git a/docs/sqa/reviews/rc-016-create-project-sh.md b/docs/sqa/reviews/rc-016-create-project-sh.md new file mode 100644 index 0000000..4289b2f --- /dev/null +++ b/docs/sqa/reviews/rc-016-create-project-sh.md @@ -0,0 +1,79 @@ +# SQA Review Record: create-project.sh (MIL-001) + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-016 | +| CrossReference | [MIL-001], [QC-SH-001] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-05 | Proposed | Jens Tirsvad Nielsen | S02 | Initial version | pending | + +--- + +## Artifact Under Review + +- Instance reviewed: `create-project.sh` and `tests/` on branch `mil-001-foundation` (reviewed at `102dd24` plus the fixes listed below), the deliverable of [MIL-001] +- Checklist used: [QC-SH-001] +- Review date: 2026-10-05 +- Tool versions: bash 5.2.37, shellcheck 0.11.0, shfmt 3.14.1 (Windows, Git Bash) + +## Checklist Results + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | Starts with `#!/usr/bin/env bash` and `set -euo pipefail` (or a comment explains the exception) | Pass | `set -Eeuo pipefail` follows the header comment. | +| 2 | Every expansion is quoted; lists are arrays; tests use `[[ ]]` and `$(...)` | Pass | `shellcheck` is clean; no backticks or `[ ]`. | +| 3 | Names follow the conventions: `kebab-case.sh` files, `snake_case` functions and variables, `UPPER_SNAKE` constants and environment variables | Pass | Fixed during this review: the boolean keys `use_github` and `plan_gate` were renamed `has_github` and `is_plan_gate_enabled` (the `is_` / `has_` rule). | +| 4 | Passes `shellcheck` and `bash -n` with no unexplained `disable` comments | Pass | Clean with the tools above. Every `disable` carries its reason: SC2034 (namerefs and results read by callers), SC2094 (loop only uses the file name in messages), SC2004 (associative array key), and in `tests/` SC2016 and SC2034 (literal snippet text, results read by other files). | +| 5 | Errors go to standard error with an `error:` message and a non-zero exit code; bad or missing arguments print a usage line | Pass | `die` and `usage_error` (exit 1 and 2); tests cover an unknown option and an option without a value. | +| 6 | Temporary files use `mktemp` with a `trap ... EXIT` cleanup; no fixed `/tmp` names | Pass | Private directory created with `umask 077`; files removed one by one, directory with `rmdir`. Verified after a normal run, a failed run and SIGTERM (test added). SIGINT was not verified: see the action items. | +| 7 | No secret is written in the script, echoed, or put on a command line; secrets come from the environment or a gitignored file | Pass | Fixed during this review: under `bash -x` the script printed the token 47 times in the trace. Tracing is now switched off with a warning, and a test fails if the guard is removed (checked by mutation). Tokens go through a private curl config file, never the command line; output is redacted; error messages name the key and line, never the value. | +| 8 | A script that changes state outside its own directory defaults to a dry run or needs an explicit flag, and says so in its header | Pass | This version contacts no host and changes nothing; the header says so. It only creates a private temporary directory, removed on exit. | +| 9 | A header comment states purpose, usage, options, environment variables and exit codes | Pass | All present, plus files, requirements, tracing and what the script implements; `--help` prints it. | +| 10 | The script implements a task or design it cites; deviations are recorded | Pass | Fixed during this review: the header now cites MIL-001 tasks 1 to 6 (issues #3 to #8), US-001.01 and UC-001, and records the one deviation (the second `GITEA_URL` key is `GITEA_API_URL`). | +| 11 | Behaviour is tested for success, failure and any disabled or bypass path | Pass | 209 checks: successful runs with and without GitHub, bad input, missing tools, network failure, redaction, tracing, termination. The script has no bypass flag. Mutation checks: a planted token leak and the removed tracing guard were both caught. | +| 12 | Formatted with `shfmt` (or the project's formatter) | Pass | `shfmt -i 2 -ci` reports no difference. | +| 13 | Safe to re-run: a second run does not duplicate or corrupt what the first did | Pass | The script keeps no state. | +| 14 | Bash version and external tools it needs are stated; GNU-only options are named | Pass | Fixed during this review: the header now lists bash 4.4, git, curl, mktemp, optional jq, the base tools it calls and the GNU or BSD `stat` form. | + +## Defects found and fixed during this review + +| Defect | Fix | Test | +| --- | --- | --- | +| `bash -x` printed the tokens in the trace | Tracing is switched off with a warning | `test_tracing_does_not_leak_secrets` | +| A byte order mark on the first line of a config file gave an unclear "expected KEY=VALUE" error | The mark is ignored | `test_byte_order_mark_is_accepted` | +| `jq` on Windows added a carriage return to every value `json_get` returned | The carriage return is stripped | `test_json_get_with_and_without_jq` | +| `json_get` without `jq` returned the last occurrence of a key on one-line JSON | It returns the first occurrence | `test_json_get_with_and_without_jq` | + +## MIL-001 Go/No-Go check + +| # | Criterion | Result | +| --- | --- | --- | +| 1 | `shellcheck create-project.sh` reports no errors | Go: clean | +| 2 | Neither config file is `source`d; unknown keys and malformed lines are rejected | Go: parser tests, including values that would run a command | +| 3 | No token appears in stdout, stderr or a log in any test, including failure paths | Go: end-to-end and trace tests | +| 4 | Missing `git` or `curl` stops the script before any change | Go: empty `PATH` test, nothing created | +| 5 | `.env` is ignored by git; both example files contain placeholders only | Go: tested | +| 6 | All acceptance criteria of US-001.01 are met | Go: validation without execution, stop on a missing tool or bad value, prompts for every detail | + +## Overall Verdict + +Go — All mandatory criteria of QC-SH-001 pass after the fixes above, and all six MIL-001 Go/No-Go criteria are met. Author and reviewer are the same person for now (S01 and S02 are both held by the Maintainer), so the framework independence rule is not met; re-review when a second person takes S02. + +## Action Items + +These are follow-ups, not conditions on the Go. + +| Action | Owner | Due | +| --- | --- | --- | +| Run `tests/run-tests.sh` on Linux and macOS (bash 4.4 or later), including the `.env` permission warning, which is skipped on Windows | S02 | 2026-10-30 | +| Check by hand that Ctrl-C removes the temporary directory (a background test cannot send SIGINT) | S02 | 2026-10-30 | +| Check the repository name rules of GitHub and Gitea in the MIL-002 preflight; the script only checks a common safe subset | S02 | 2026-10-30 | + +--- + +[MIL-001]: ../milestones/mil-001-foundation.md +[QC-SH-001]: ../../../framework/qc/qc-programming-shell.md diff --git a/docs/sqa/traceability-matrix.md b/docs/sqa/traceability-matrix.md index 4e1e601..9a710f8 100644 --- a/docs/sqa/traceability-matrix.md +++ b/docs/sqa/traceability-matrix.md @@ -26,7 +26,7 @@ updated whenever an artifact instance is created or reviewed. | [BC-001] | BC | - | [SA-001], [PP-001], [MIL-001], [MIL-002], [MIL-003], [US-001], [UCD-001] | [RC-010] | | [SA-001] | SA | [BC-001] | [UCD-001], [UC-001], [DICT-001] | [RC-013] | | [PP-001] | PP | [BC-001], [SA-001] | [MIL-001], [MIL-002], [MIL-003] | [RC-012] | -| [MIL-001] | MIL | [BC-001], [PP-001] | [US-001] | [RC-011] | +| [MIL-001] | MIL | [BC-001], [PP-001] | [US-001] | [RC-011], [RC-016] | | [MIL-002] | MIL | [BC-001], [PP-001] | [US-001] | [RC-014] | | [MIL-003] | MIL | [BC-001], [PP-001] | [US-001] | [RC-015] | | [UCD-001] | UCD | [BC-001], [SA-001] | [US-001], [UC-001] | [RC-009] | @@ -76,4 +76,5 @@ updated whenever an artifact instance is created or reviewed. [RC-013]: ./reviews/rc-013-sa-001.md [RC-014]: ./reviews/rc-014-mil-002.md [RC-015]: ./reviews/rc-015-mil-003.md +[RC-016]: ./reviews/rc-016-create-project-sh.md [02875ae]: https://git.tirsystem.com/TirSystem-BashScript/repo_foundry/commit/02875aee5f2953473924074eea0056eb31af6b7a diff --git a/tests/lib.sh b/tests/lib.sh index 2f120c6..66c3907 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -83,7 +83,7 @@ new_workdir() { # first, then the now empty directories from the bottom up. remove_workdir() { if [[ -n $WORK && -d $WORK ]]; then - find "$WORK" -type f -delete + find "$WORK" \( -type f -o -type p \) -delete find "$WORK" -depth -type d -exec rmdir {} + fi WORK="" diff --git a/tests/test-prompts.sh b/tests/test-prompts.sh index 7762925..6749094 100644 --- a/tests/test-prompts.sh +++ b/tests/test-prompts.sh @@ -54,17 +54,17 @@ EOF test_collect_details_with_github() { run_lib $'my-app\nA test app\npublic\nTirSystem\ny\nmy-org\n\ny\n' \ 'collect_project_details -for k in name description visibility gitea_owner use_github github_owner directory plan_gate; do +for k in name description visibility gitea_owner has_github github_owner directory is_plan_gate_enabled; do printf "%s=%s\n" "$k" "${PROJECT[$k]}" done' assert_status "details collected" 0 "$STATUS" - assert_eq "details" $'name=my-app\ndescription=A test app\nvisibility=public\ngitea_owner=TirSystem\nuse_github=1\ngithub_owner=my-org\ndirectory=./my-app\nplan_gate=1' "$OUT" + assert_eq "details" $'name=my-app\ndescription=A test app\nvisibility=public\ngitea_owner=TirSystem\nhas_github=1\ngithub_owner=my-org\ndirectory=./my-app\nis_plan_gate_enabled=1' "$OUT" } test_collect_details_without_github() { run_lib $'my-app\n\n\nTirSystem\nn\n\nn\n' \ 'collect_project_details -printf "%s|%s|%s|%s\n" "${PROJECT[visibility]}" "${PROJECT[use_github]}" "[${PROJECT[github_owner]}]" "${PROJECT[plan_gate]}"' +printf "%s|%s|%s|%s\n" "${PROJECT[visibility]}" "${PROJECT[has_github]}" "[${PROJECT[github_owner]}]" "${PROJECT[is_plan_gate_enabled]}"' assert_status "GitHub skipped" 0 "$STATUS" assert_eq "defaults and no GitHub owner" "private|0|[]|0" "$OUT" assert_not_contains "no GitHub owner prompt" "$ERR" "GitHub owner" diff --git a/tests/test-security.sh b/tests/test-security.sh index 7d1d90b..a26cb16 100644 --- a/tests/test-security.sh +++ b/tests/test-security.sh @@ -123,6 +123,54 @@ test_script_uses_no_unsafe_constructs() { assert_not_contains "no eval" "$code" "eval " assert_not_contains "no source" "$code" "source " assert_not_contains "no dot-source" "$code" $'\n. ' - assert_not_contains "no set -x" "$code" "set -x" + check + if grep -Eq '^[[:space:]]*set -[A-Za-z]*x' <<<"$code"; then + fail "the script turns tracing on" + fi assert_not_contains "no fixed /tmp file" "$code" "/tmp/file" } + +test_tracing_does_not_leak_secrets() { + # bash -x would print every assignment and command, secrets included, so + # the script switches tracing off and says so. + write_fixtures + STATUS=0 + PATH="$WORK/bin:$PATH" TMPDIR="$WORK/tmp" "$BASH" -x "$SCRIPT" \ + --config "$WORK/config.env" --env "$WORK/.env" <<<"$ANSWERS_GITHUB" \ + >"$WORK/out.txt" 2>"$WORK/err.txt" || STATUS=$? + assert_status "run under bash -x" 0 "$STATUS" + assert_contains "tracing disabled" "$(cat "$WORK/err.txt")" "tracing (set -x) is disabled" + assert_not_contains "no Gitea token in the trace" "$(cat "$WORK/err.txt" "$WORK/out.txt")" "$FAKE_GITEA_TOKEN" + assert_not_contains "no GitHub token in the trace" "$(cat "$WORK/err.txt" "$WORK/out.txt")" "$FAKE_GITHUB_PAT" +} + +test_byte_order_mark_is_accepted() { + printf '\xef\xbb\xbfGITEA_URL=https://a.test\nGITHUB_WEB_URL=https://b.test\n' >"$WORK/c.env" + run_lib "" "parse_env_file \"$WORK/c.env\" CONFIG_KEYS CONFIG +echo \"\${CONFIG[GITEA_URL]}\"" + assert_status "BOM on the first line" 0 "$STATUS" + assert_eq "first key read" "https://a.test" "$OUT" +} + +test_termination_removes_temp_files() { + write_fixtures + if ! mkfifo "$WORK/in" 2>/dev/null; then + return 0 + fi + PATH="$WORK/bin:$PATH" TMPDIR="$WORK/tmp" "$BASH" "$SCRIPT" \ + --config "$WORK/config.env" --env "$WORK/.env" <"$WORK/in" \ + >/dev/null 2>&1 & + local pid=$! tries=0 + # Keep the pipe open so the script waits at its first prompt. + exec 7>"$WORK/in" + while [[ -z "$(find "$WORK/tmp" -mindepth 1)" ]] && ((tries < 50)); do + sleep 0.1 + tries=$((tries + 1)) + done + assert_eq "temp directory exists while running" 1 "$(find "$WORK/tmp" -mindepth 1 | wc -l | tr -d ' ')" + kill -TERM "$pid" + # wait returns the signal status (143); only the cleanup matters here. + wait "$pid" 2>/dev/null || true + exec 7>&- + assert_eq "temp directory removed after SIGTERM" "" "$(find "$WORK/tmp" -mindepth 1)" +}