aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

feat(ci): add shellcheck linting for shell scripts to the toolchain

Open
#278 0 comments 0 reactions 0 assignees View on GitHub
ci-cd enhancement
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

## Problem

The repo statically analyzes every language surface **except shell**:

| Surface | Linter | In CI / hooks? |
|---|---|---|
| TypeScript | eslint | ✅ |
| Python (agent) | semgrep + agent quality | ✅ |
| GitHub Actions | zizmor | ✅ |
| Secrets / deps | gitleaks / osv-scanner / grype / retire | ✅ |
| **Shell scripts** | **— none —** | ❌ |

`grep shellcheck` across `mise.toml`, `.github/`, and all package `mise.toml` files returns nothing. Yet there are **5 first-party shell scripts** in-tree today:

- `agent/run.sh` — **ships inside the deployed container image**
- `agent/prepare-commit-msg.sh` — **ships inside the deployed container image**
- `agent/scripts/create-local-tables.sh`
- `scripts/ci-build.sh` — runs the full monorepo build (mirrors CI)
- `scripts/cleanup-ephemeral-stacks.sh` — **destructive** (deletes CloudFormation stacks + ENIs); added in PR #109 (#72)

Shell is therefore a maintained — and partly *shipped* and *destructive* — surface with **zero** static analysis. The cleanup script in PR #109 made this gap concrete: its review relied on a `shellcheck` binary fetched ad hoc via `mise x`, not on any committed project gate, so the next shell change has nothing protecting it.

## Why now

A defense-in-depth shell linter catches the exact class of bug that bit the cleanup script before hardening — fail-open logic, unquoted expansions, `set -e` foot-guns (SC2086, SC2046, SC2164, etc.) — none of which semgrep's current rule set flags for bash.

## Proposed change

1. Add `shellcheck` to the root `mise.toml` `[tools]` block (pin a version, consistent with `gitleaks`/`semgrep`/`zizmor`).
2. Add a `security:shell` (or `lint:shell`) mise task that runs shellcheck over first-party scripts, **excluding** generated trees (`cdk/cdk.out/**`, `node_modules/**`).
3. Wire it into the `security` aggregate task and the prek **pre-commit**/**pre-push** hooks (matching how the other linters are wired).
4. Add it to the CI workflow alongside the existing security/lint steps.

## Scoping note (already measured)

Running shellcheck against the existing scripts today:

- **0 error-level findings** across all 5 scripts.
- Only **1 info-level** finding (SC2016 in `agent/run.sh`, a JMESPath/literal-backtick false positive — same pattern already annotated with a line-scoped `# shellcheck disable=SC2016` in `cleanup-ephemeral-stacks.sh`).

So a gate at `--severity error` (or `warning`) lands **without a backlog of pre-existing failures** — low-risk to introduce.

## Acceptance criteria

- [ ] `shellcheck` pinned in root `mise.toml` `[tools]`
- [ ] mise task lints all first-party `*.sh`, excludes generated/vendored trees
- [ ] Task added to the `security` aggregate and to prek pre-commit + pre-push hooks
- [ ] CI runs the shell lint step
- [ ] Chosen severity threshold documented (proposal: `error`, or `warning` if the team wants stricter)
- [ ] Existing scripts pass at the chosen threshold (annotate the one SC2016 false positive if `warning`/`info` is chosen)
- [ ] AGENTS.md "Commands you can use" updated with the new task

## References

- PR #109 (#72) — the destructive script that surfaced this gap
- Existing patterns to mirror: `security:gh-actions` (zizmor), `security:sast` (semgrep) in root `mise.toml`

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.