aws-samples / aws-samples/sample-autonomous-cloud-coding-agents
feat(ci): add shellcheck linting for shell scripts to the toolchain
- 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
Assessment
This issue has not been assessed yet.