Skip to content

SonarCloud: shell script hygiene #1193

Open

Description

@don-petry

SonarCloud: shell script hygiene

964 open SonarCloud finding(s) in combsmith/.github, worst severity MAJOR. Generated by the org SonarCloud Audit.

View in SonarCloud →

Findings

Rule Count Representative message
shelldre:S7688 964 Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more featur

Affected files

  • scripts/compliance-audit.sh (183)
  • scripts/lib/agent-rate-limit.sh (107)
  • scripts/lib/canary-rollout.sh (71)
  • scripts/compliance-remediate.sh (38)
  • scripts/agent-rate-limit-gate.sh (36)
  • scripts/lib/push-protection.sh (34)
  • scripts/migrate-major-channels.sh (33)
  • scripts/apply-repo-settings.sh (30)
  • .github/scripts/add-to-project/classify-initiative.sh (26)
  • scripts/agents-md-cycle-log.sh (26)
  • scripts/sonarcloud-audit.sh (24)
  • scripts/apply-rulesets.sh (22)
  • scripts/automerge-standards-sync.sh (20)
  • scripts/standards-staleness.sh (20)
  • scripts/lib/gitignore-baseline.sh (19)
  • scripts/lib/standards-release.sh (19)
  • .github/scripts/add-to-project/lib.sh (18)
  • .github/scripts/feature-ideation/lib/gh-safe.sh (15)
  • scripts/compliance-retrigger.sh (15)
  • scripts/fix-ruleset-bypass.sh (13)

Priority

Labeled priority:major — mapped from this bucket's worst SonarCloud severity (MAJOR).

Acceptance

  • All findings in this workstream resolved to zero in SonarCloud
  • No behavior change; CI green
  • Real fixes (no blanket NOSONAR unless a confirmed false positive, noted inline)

Idempotent issue — updated automatically each audit run; auto-closed when the finding count reaches zero.

Activity

  1. don-petry commented on Sep 29, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead Implementation Plan

    Issue: #1193 — SonarCloud: shell script hygiene

    Scope

    Resolve shelldre shell-script hygiene findings across first-party scripts (scripts/**, .github/scripts/**) as behavior-preserving refactors, and exclude vendored third-party code (node_modules/**) from SonarCloud analysis so its findings stop being reported. No behavior change; existing bats suites are the regression net.

    Approach & Priority (largest, safest wins first)

    1. Vendored-code exclusion (config). The two node_modules/bats/lib/bats-core/*.bash files (55 findings) plus the S7684 (BATS_FORMATTER_TEST_DURATION) and S100 (bats_export_preprocess_source_*) findings all originate in vendored bats. Add sonar.exclusions=node_modules/** to the root sonar-project.properties — third-party code must not be edited or audited as ours.
    2. S7688 [ → [[ (898 findings, dominant). Confirmed safe here: no binary -a/-o, no escaped \</\>, no unquoted-glob RHS in single brackets. Convert first-party test commands via a reviewed transformation, gated by shellcheck + bats.
    3. Smaller mechanical rules: S7677 redirect errors to stderr (8), S1481 unused locals (2), S1135 TODO (2), S1066 merge nested if (5).
    4. Semantic rules where safely doable: S131 default *) case (26), S7679 positional-param → local (86), S7682 explicit return (149, using bare return to preserve exit status), S1192 repeated-literal constants (28).

    Implementation Checklist

    • sonar-project.properties — add sonar.exclusions=node_modules/**
    • [[ conversion across all first-party scripts/** + .github/scripts/** bash scripts
    • stderr redirects, unused-local removals, TODO/merge-if cleanups
    • default-case / local / explicit-return / constant fixes per file, verified against bats
    • git diff review of every changed line

    Tests to Write

    • None (behavior-preserving refactor + config — Phase 2 exception). The existing bats suites under tests/ and test/scripts/** are the regression net and must stay green.

    Test Command

    bats <affected .bats files> (via node_modules/.bin/bats) + bash .dev-lead/scripts/dev-lead-lint.sh

  2. don-petry commented on Sep 29, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead: Progress (in progress — not yet durable)

    Provisional. Nothing is pushed yet; the automation posts the authoritative
    completion record (with PR number + head SHA) after the push. Treat the notes
    below as intent, not delivered results.

    Plan Execution (intended)

    • Vendored-code exclusion (config). Added sonar.exclusions=node_modules/** to the root sonar-project.properties. The bats runner is a package.json dependency; its bundled bats-core/*.bash files are the source of the two node_modules/** file entries plus the S7684 (BATS_FORMATTER_TEST_DURATION) and S100 (bats_export_preprocess_source_*) findings — third-party code we should not edit or audit as ours.
    • S7688 [ → [[ (the dominant rule). Converted single-bracket test commands to [[ … ]] across every first-party bash script under scripts/** and .github/scripts/**, via a quote/heredoc/comment-aware scanner so brackets inside jq/awk programs, JSON, heredoc bodies, and comments are never touched. Existing [[ ]], ${arr[i]}, globs, and case patterns are left alone.

    Approach to "no behavior change"

    • Converted brackets are all -n/-z/=/!=/-eq/-lt/… tests that behave identically in [ and [[. Confirmed there are no binary -a/-o, escaped \</\>, or unquoted-glob RHS in single brackets, and existing [[ … =~ … ]] regex tests were left untouched (only their single-bracket siblings changed).

    Local check output (provisional — not the pushed run)

    bash -n on all 57 first-party scripts: all pass
    shellcheck --severity=warning -x (scripts/** + .github/scripts/**): clean
    .dev-lead/scripts/dev-lead-lint.sh: all checks passed
    bats (103 files): the only failing tests are 5 pre-existing DRY_RUN cases in
      tests/test_bootstrap_new_repo.bats that fail identically on the unmodified
      scripts (environmental — need gh/network in this sandbox). Verified by
      git-stashing the changes and re-running: same 5 fail before and after, so
      this change introduces no regressions.
    

    Files touched (this workspace, not yet pushed)

    • sonar-project.properties: add node_modules/** exclusion.
    • 52 shell scripts under scripts/** and .github/scripts/**: [ → [[ test-command conversion only (no logic changes).

    Notes / deferred (transparent scope)

    • Fully addressed here: S7688 (dominant) and the vendored-code findings (S7684, S100, and the two node_modules files).
    • Deferred: S7682 (explicit return), S7679 (positional→local), S1192 (repeated-literal constants), S131 (default case), S1066 (merge if), S1481 (unused local), S1135 (TODO), S7677 (stderr). Rationale: these require per-location semantic judgment; the shelldre analyzer is not available locally to pinpoint or confirm resolution, and blanket edits (e.g. adding return/locals to ~235 sites, or completing TODOs) carry behavior-change risk against the hard "no behavior change" criterion. shellcheck reports zero unused locals (S1481) in these files, and the TODO matches are descriptive comments in compliance-audit.sh, not actionable tasks. Because this issue is idempotent, the residual will re-report for a focused follow-up.
  3. don-petry commented on Sep 29, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead: Implementation Complete — PR #1196

    The implementation for issue #1193 is durable: commits are pushed and a pull request is open. This record is backed by verifiable artifacts you can confirm in one click.

    Review happens on the PR. Acceptance criteria and test results are evidenced by the PR's diff and CI checks — not asserted here. If a later stage fails, this claim is retracted in place.

  4. added a commit that references this issue on Oct 3, 2026
  5. don-petry commented on Oct 6, 2026

    @don-petry
    ContributorAuthor

    SonarCloud Audit (2026-10-06): still open — 1260 finding(s), worst severity CRITICAL.

  6. added 2 commits that reference this issue on Oct 11, 2026
  7. don-petry commented on Oct 13, 2026

    @don-petry
    ContributorAuthor

    SonarCloud Audit (2026-10-13): still open — 1296 finding(s), worst severity CRITICAL.

  8. added and removed on Oct 20, 2026
  9. don-petry commented on Oct 20, 2026

    @don-petry
    ContributorAuthor

    SonarCloud Audit (2026-10-20): still open — 964 finding(s), worst severity MAJOR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dev-leadFor dev-lead agent pickuppriority:majorSonarCloud MAJOR severitysonarcloud-auditSonarCloud audit finding (idempotent marker)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions