Skip to content

ci: run shellcheck, and fix the three issues it found - #13

Open
presto8 wants to merge 1 commit into
mainfrom
ci/shellcheck
Open

presto8 wants to merge 1 commit into
mainfrom
ci/shellcheck

Conversation

@presto8

@presto8 presto8 commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Adds a GitHub Actions job that shellchecks the bash scripts (setup,
create-gruvbox) and syntax-checks the zsh files with 'zsh -n'.
shellcheck cannot parse zsh, so those are only syntax-checked.

The three findings on setup, all fixed here:

SC2174 (warning) 'mkdir -p "$HOME"/.ssh --mode 700' -- with -p, -m
applies only to the deepest directory created, and not at all when
the directory already exists. ~/.ssh could therefore end up with
default permissions, which ssh refuses to use. Split into mkdir -p
followed by an unconditional chmod 700.

SC2086 unquoted $HOME/.gitconfig in setup_git.

SC2016 '#https_proxy=$http_proxy' is intentionally single-quoted --
it writes a commented template line to /etc/environment -- so it
gets an explanatory disable directive rather than a change.

Note shellcheck is narrower than I first assumed: it does not catch the
logic bugs fixed on the other branches (a destination-less git clone,
'-f' against a directory, '-d' against a file), since all are
well-formed shell. Verified locally with shellcheck 0.10.0: clean exit
on both scripts.


🤖 Generated with Claude Code

Adds a GitHub Actions job that shellchecks the bash scripts (setup,
create-gruvbox) and syntax-checks the zsh files with 'zsh -n'.
shellcheck cannot parse zsh, so those are only syntax-checked.

The three findings on setup, all fixed here:

  SC2174 (warning) 'mkdir -p "$HOME"/.ssh --mode 700' -- with -p, -m
    applies only to the deepest directory created, and not at all when
    the directory already exists. ~/.ssh could therefore end up with
    default permissions, which ssh refuses to use. Split into mkdir -p
    followed by an unconditional chmod 700.

  SC2086 unquoted $HOME/.gitconfig in setup_git.

  SC2016 '#https_proxy=$http_proxy' is intentionally single-quoted --
    it writes a commented template line to /etc/environment -- so it
    gets an explanatory disable directive rather than a change.

Note shellcheck is narrower than I first assumed: it does not catch the
logic bugs fixed on the other branches (a destination-less git clone,
'-f' against a directory, '-d' against a file), since all are
well-formed shell. Verified locally with shellcheck 0.10.0: clean exit
on both scripts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new workflow’s zsh syntax-check step doesn’t cover all zsh files it claims to (e.g., p10k.zsh and zpreztorc), leaving gaps in CI enforcement.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds CI linting for shell scripts and applies the three ShellCheck-driven fixes in setup to harden permissions and quoting.

Changes:

  • Fix ~/.ssh permissions handling by separating mkdir -p from an unconditional chmod 700.
  • Quote $HOME/.gitconfig path and quote /etc/environment in grep calls; document intentional single-quoted proxy template line with a ShellCheck disable.
  • Add a GitHub Actions workflow to run shellcheck on bash scripts and zsh -n syntax checks on zsh files.
File summaries
File Description
setup Addresses ShellCheck findings around directory permissions, quoting, and intentional literal output.
.github/workflows/shellcheck.yml Introduces CI steps to lint bash scripts and syntax-check zsh scripts.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +22 to +26
- name: Check zsh syntax
run: |
sudo apt-get install -y zsh
zsh -n prestobuntu.zsh
zsh -n tmux-new-session.plugin.zsh
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants