From 5ce623eed5ba347768db88f6185e0e8a8fdc3351 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 16:04:10 -0400 Subject: [PATCH 1/4] fix(git-check): isolate per-repo work in subshell (SUR-2470) Run each repository scan in a subshell and pass bucket+record back to the parent so cd failures and iteration order never leave the outer shell in another repo. Add bats coverage that cwd is restored. --- bash/git-check | 103 +++++++++++++++++++++++++++---------------- tests/git-check.bats | 10 +++++ 2 files changed, 75 insertions(+), 38 deletions(-) diff --git a/bash/git-check b/bash/git-check index eec433f..260f1e5 100755 --- a/bash/git-check +++ b/bash/git-check @@ -134,48 +134,75 @@ declare -a UNCOMMITTED=() declare -a RELEASABLE=() declare -a DEVELOPMENT=() for repo in $(find "${base_path}${EXTRA}" -mindepth "${DEPTH}" -maxdepth "${DEPTH}" -name .git -exec dirname {} \; | sort); do - cd "$repo" || exit 1 - base_name=$(base_name_for_repo "$base_path" "$repo") - orgname=$(dirname "$base_name") - branch_name=$(get_branch_name) - dirty_version=$(git::describe --tags 2>/dev/null)-dirty - version=$(git::describe --dirty --tags 2>/dev/null) - long_version=$(git::describe --long --tags 2>/dev/null) + # Per-repo work runs in a subshell so a failed or partial cd never leaves + # the outer script in another repository's directory (SUR-2470). + block=$( + cd "$repo" || exit 1 + base_name=$(base_name_for_repo "$base_path" "$repo") + orgname=$(dirname "$base_name") + branch_name=$(get_branch_name) + dirty_version=$(git::describe --tags 2>/dev/null)-dirty + version=$(git::describe --dirty --tags 2>/dev/null) + long_version=$(git::describe --long --tags 2>/dev/null) + + status=$(git::cmd status --short | wc -l) + fetch_if_possible "$base_name" + if [ "$status" = "0" ]; then + pull_if_different "$base_name" "$branch_name" + else + log::notice "Examining $base_name: dirty" + fi - len=${#base_name} - branch_len=${#branch_name} + if [ -z "$CLIENT" ]; then + pr_count=$(get_gh_pr_count "$orgname" "$base_name") + elif [ "$CLIENT" = "TASE" ]; then + pr_count=$(get_bb_pr_count "$CLIENT" "$base_name") + else + pr_count='.' + fi + # Tab-delimited record: name TAB pr_count TAB branch TAB version. Tabs + # are forbidden in any of these fields (repo paths and branch names use + # forward-slashes, not tabs), so the encoding is unambiguous. + if [ -z "$version" ]; then + version=$(git::cmd rev-parse --short HEAD) + printf '%s\n' UNCOMMITTED + printf '%s\n' "$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")" + elif [ "$version" = "$dirty_version" ]; then + printf '%s\n' UNCOMMITTED + printf '%s\n' "$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")" + elif [ "$version" != "$long_version" ]; then + printf '%s\n' RELEASABLE + printf '%s\n' "$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")" + else + printf '%s\n' DEVELOPMENT + printf '%s\n' "$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")" + fi + ) || exit 1 + + bucket=${block%%$'\n'*} + record=${block#*$'\n'} + case "$bucket" in + UNCOMMITTED) + UNCOMMITTED+=("$record") + ;; + RELEASABLE) + RELEASABLE+=("$record") + ;; + DEVELOPMENT) + DEVELOPMENT+=("$record") + ;; + *) + log::error "git-check: unexpected bucket from repo processing: $bucket" + exit 1 + ;; + esac + + IFS=$'\t' read -r name _ branch _ <<<"$record" + len=${#name} + branch_len=${#branch} max_repo_len=$((len > max_repo_len ? len : max_repo_len)) max_branch_len=$((branch_len > max_branch_len ? branch_len : max_branch_len)) - status=$(git::cmd status --short | wc -l) - fetch_if_possible "$base_name" - if [ "$status" = "0" ]; then - pull_if_different "$base_name" "$branch_name" - else - log::notice "Examining $base_name: dirty" - fi - - if [ -z "$CLIENT" ]; then - pr_count=$(get_gh_pr_count "$orgname" "$base_name") - elif [ "$CLIENT" = "TASE" ]; then - pr_count=$(get_bb_pr_count "$CLIENT" "$base_name") - else - pr_count='.' - fi - - # Tab-delimited record: name TAB pr_count TAB branch TAB version. Tabs - # are forbidden in any of these fields (repo paths and branch names use - # forward-slashes, not tabs), so the encoding is unambiguous. - if [ -z "$version" ]; then - version=$(git::cmd rev-parse --short HEAD) - UNCOMMITTED+=("$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")") - elif [ "$version" = "$dirty_version" ]; then - UNCOMMITTED+=("$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")") - elif [ "$version" != "$long_version" ]; then - RELEASABLE+=("$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")") - else - DEVELOPMENT+=("$(printf '%s\t%s\t%s\t%s' "$base_name" "$pr_count" "$branch_name" "$version")") - fi done function output() { diff --git a/tests/git-check.bats b/tests/git-check.bats index 8658047..22e2aad 100644 --- a/tests/git-check.bats +++ b/tests/git-check.bats @@ -100,3 +100,13 @@ init_repo() { echo "$clean" | grep -E 'org/repo' echo "$clean" | grep -E 'v0\.3\.0-[0-9]+-g' } + +@test "git-check restores caller working directory after scanning repos (SUR-2470)" { + run bash -c " + cd /tmp || exit 1 + before=\$PWD + '$GIT_CHECK' -b testbase >/dev/null 2>&1 + [ \"\$PWD\" = \"\$before\" ] + " + [ "$status" -eq 0 ] +} From 5e8470b1cae403f0173f44aa1a90818e5da0a8de Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 16:06:20 -0400 Subject: [PATCH 2/4] test(aws): cover refresh_scan cache window and jq floor (SUR-2477) Add bats cases for COMPLETE short-circuit, stale scan re-trigger, non-COMPLETE path, and fractional imageScanCompletedAt via mocked _describe_findings, date, and scan_image. --- tests/aws.bats | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/aws.bats b/tests/aws.bats index 1d4696c..f710061 100644 --- a/tests/aws.bats +++ b/tests/aws.bats @@ -94,3 +94,73 @@ setup() { # refresh_scan must NOT have been called — tag '1.2.3' is not '1X2X3' literally [[ "$output" != *"CALLED"* ]] } + +# SUR-2477: aws::refresh_scan cache window and jq floor on fractional timestamps + +@test "aws::refresh_scan skips aws::scan_image when scan is COMPLETE and within cache window (SUR-2477)" { + run bash -c " + export LOG_DISABLE_INFO=true + source '$REPO_ROOT/bash/includer.sh' + @include aws + date() { printf '%s\n' '2000000000'; } + aws::_describe_findings() { + printf '%s\n' '{\"imageScanStatus\":{\"status\":\"COMPLETE\"},\"imageScanFindings\":{\"imageScanCompletedAt\":1999990000.987}}' + } + aws::scan_image() { echo SCAN_IMAGE_CALLED; return 0; } + aws::refresh_scan myrepo mytag 7 + " + [ "$status" -eq 0 ] + [[ "$output" != *"SCAN_IMAGE_CALLED"* ]] +} + +@test "aws::refresh_scan calls aws::scan_image when COMPLETE scan is older than cache window (SUR-2477)" { + run bash -c " + export LOG_DISABLE_INFO=true + source '$REPO_ROOT/bash/includer.sh' + @include aws + date() { printf '%s\n' '2000000000'; } + aws::_describe_findings() { + printf '%s\n' '{\"imageScanStatus\":{\"status\":\"COMPLETE\"},\"imageScanFindings\":{\"imageScanCompletedAt\":1000.1}}' + } + aws::scan_image() { echo SCAN_IMAGE_CALLED; return 0; } + aws::refresh_scan myrepo mytag 7 + " + [ "$status" -eq 0 ] + [[ "$output" == *"SCAN_IMAGE_CALLED"* ]] +} + +@test "aws::refresh_scan calls aws::scan_image when scan is not COMPLETE (SUR-2477)" { + run bash -c " + export LOG_DISABLE_INFO=true + source '$REPO_ROOT/bash/includer.sh' + @include aws + date() { printf '%s\n' '2000000000'; } + aws::_describe_findings() { + printf '%s\n' '{\"imageScanStatus\":{\"status\":\"IN_PROGRESS\"}}' + } + aws::scan_image() { echo SCAN_IMAGE_CALLED; return 0; } + aws::refresh_scan myrepo mytag 7 + " + [ "$status" -eq 0 ] + [[ "$output" == *"SCAN_IMAGE_CALLED"* ]] +} + +@test "aws::refresh_scan uses jq floor so fractional scan time does not spuriously refresh (SUR-2477)" { + run bash -c " + export LOG_DISABLE_INFO=true + source '$REPO_ROOT/bash/includer.sh' + @include aws + # Window edge: earliest = 2000000000 - 604800 = 1999395200 + # Integer part 1999395200 is not fresh; 1999395200.001 floor is still 1999395200, + # so earliest < completedAt is false — would refresh without careful equality. + # Use completedAt just above earliest as integer: 1999395201.999 floors to 1999395201. + date() { printf '%s\n' '2000000000'; } + aws::_describe_findings() { + printf '%s\n' '{\"imageScanStatus\":{\"status\":\"COMPLETE\"},\"imageScanFindings\":{\"imageScanCompletedAt\":1999395201.999}}' + } + aws::scan_image() { echo SCAN_IMAGE_CALLED; return 0; } + aws::refresh_scan myrepo mytag 7 + " + [ "$status" -eq 0 ] + [[ "$output" != *"SCAN_IMAGE_CALLED"* ]] +} From e6f153c0597c23f3bc563e9e4c96b1ed4b46089d Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 16:08:26 -0400 Subject: [PATCH 3/4] test(docker): cover repo_tags_has and cp_if_different short-circuit (SUR-2478) Exercise same-image vs different-image flows with mocked inspect output and counters for tag/push side effects. --- tests/docker.bats | 87 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/tests/docker.bats b/tests/docker.bats index 24679e9..bd36160 100644 --- a/tests/docker.bats +++ b/tests/docker.bats @@ -157,3 +157,90 @@ run_cp() { [ "$status" -ne 0 ] [[ "$output" == *"No docker credentials"* ]] || [[ "$output" == *"docker login"* ]] } + +# SUR-2478: docker::repo_tags_has and docker::cp_if_different same-image path + +@test "docker::repo_tags_has returns 0 when inspect lists exact destination tag (SUR-2478)" { + run bash -c " + export LOG_DISABLE_INFO=true LOG_DISABLE_DEBUG=true LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include docker + docker::inspect() { + printf '%s\n' '[{\"RepoTags\":[\"registry.example/a:v1\",\"registry.example/b:v2\"]}]' + } + docker::repo_tags_has 'registry.example/a:v1' 'registry.example/b:v2' + " + [ "$status" -eq 0 ] +} + +@test "docker::repo_tags_has returns 0 when tag matches after stripping index.docker.io prefix (SUR-2478)" { + run bash -c " + export LOG_DISABLE_INFO=true LOG_DISABLE_DEBUG=true LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include docker + docker::inspect() { + printf '%s\n' '[{\"RepoTags\":[\"foo/bar:tag\"]}]' + } + docker::repo_tags_has 'foo/bar:tag' 'index.docker.io/foo/bar:tag' + " + [ "$status" -eq 0 ] +} + +@test "docker::repo_tags_has returns 1 when no RepoTags entry matches destination (SUR-2478)" { + run bash -c " + export LOG_DISABLE_INFO=true LOG_DISABLE_DEBUG=true LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include docker + docker::inspect() { + printf '%s\n' '[{\"RepoTags\":[\"registry.example/a:v1\"]}]' + } + docker::repo_tags_has 'registry.example/a:v1' 'registry.example/other:v9' + " + [ "$status" -ne 0 ] +} + +@test "docker::cp_if_different skips tag and push when repo_tags_has matches (SUR-2478)" { + run bash -c " + export LOG_DISABLE_INFO=true LOG_DISABLE_DEBUG=true LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include docker + docker::pull() { return 0; } + docker::inspect() { + printf '%s\n' '[{\"RepoTags\":[\"from-img\",\"to-img\"]}]' + } + tag_calls=0 + push_calls=0 + docker::tag() { tag_calls=\$((tag_calls + 1)); return 0; } + docker::push() { push_calls=\$((push_calls + 1)); return 0; } + docker::cp_if_different from-img to-img + rc=\$? + echo \"tag_calls=\$tag_calls push_calls=\$push_calls rc=\$rc\" + " + [ "$status" -eq 0 ] + [[ "$output" == *"tag_calls=0"* ]] + [[ "$output" == *"push_calls=0"* ]] + [[ "$output" == *"rc=0"* ]] +} + +@test "docker::cp_if_different pulls, tags, and pushes when images differ (SUR-2478)" { + run bash -c " + export LOG_DISABLE_INFO=true LOG_DISABLE_DEBUG=true LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include docker + docker::pull() { return 0; } + docker::inspect() { + printf '%s\n' '[{\"RepoTags\":[\"from-img\"]}]' + } + tag_calls=0 + push_calls=0 + docker::tag() { tag_calls=\$((tag_calls + 1)); return 0; } + docker::push() { push_calls=\$((push_calls + 1)); return 0; } + docker::cp_if_different from-img to-img + rc=\$? + echo \"tag_calls=\$tag_calls push_calls=\$push_calls rc=\$rc\" + " + [ "$status" -eq 0 ] + [[ "$output" == *"tag_calls=1"* ]] + [[ "$output" == *"push_calls=1"* ]] + [[ "$output" == *"rc=0"* ]] +} From 0262c63468aebab6f22cefd7ee3886e4f2ba6a85 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 16:32:37 -0400 Subject: [PATCH 4/4] test(git-check): replace vacuous PWD test with coverage note (SUR-2470) The subprocess-based $PWD assertion always passed regardless of implementation since child processes cannot modify parent PWD. Drop it; the existing multi-repo branch-column test provides the relevant indirect coverage. --- tests/git-check.bats | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/git-check.bats b/tests/git-check.bats index 22e2aad..d9a9537 100644 --- a/tests/git-check.bats +++ b/tests/git-check.bats @@ -3,6 +3,11 @@ # loop variable because $2 was assigned without `local`. Drive git-check # end-to-end against two repos on different branches and assert each # per-repo line carries the correct branch. +# +# SUR-2470: per-repo work now runs in a subshell so a failed cd never leaves +# the outer loop in another repo's directory. The multi-repo test below +# provides indirect regression coverage: if iteration state bled between repos +# the branch columns would be wrong or absent. setup() { load 'helpers.bash' @@ -100,13 +105,3 @@ init_repo() { echo "$clean" | grep -E 'org/repo' echo "$clean" | grep -E 'v0\.3\.0-[0-9]+-g' } - -@test "git-check restores caller working directory after scanning repos (SUR-2470)" { - run bash -c " - cd /tmp || exit 1 - before=\$PWD - '$GIT_CHECK' -b testbase >/dev/null 2>&1 - [ \"\$PWD\" = \"\$before\" ] - " - [ "$status" -eq 0 ] -}