Skip to content

build-lectures: drop eval, quote path inputs (#36) - #48

Merged
mmcky merged 2 commits into
mainfrom
fix/build-lectures-shell-safety
Jun 16, 2026
Merged

mmcky merged 2 commits into
mainfrom
fix/build-lectures-shell-safety

Conversation

@mmcky

@mmcky mmcky commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Closes #36 (M10, L17, L22) — shell-safety cleanup of build-lectures.

M10 — drop eval, quote the path inputs

The build command was built by string interpolation and run through eval $BUILD_CMD, so a space or shell metacharacter in source-dir/output-dir would break the build. Now it invokes jb build directly:

jb build "$SOURCE_DIR" --path-output "$OUTPUT_DIR" "${builder_args[@]}" $EXTRA_ARGS
  • source-dir/output-dir/builder/extra-args are passed via env: and the paths are quoted.
  • builder-specific flags live in a bash array (builder_args), so no eval.
  • the if/elif ladder became a case.

L17 — output-dir default ./.

Drops the cosmetic .//_build/html double-slash in the build path.

L22 — extra-args word-splitting (documented)

$EXTRA_ARGS is left unquoted on purpose so -W --keep-going splits into two flags. A comment in the step notes that quoted args with embedded spaces inside extra-args aren't supported (the standard CI caveat).

Verification

  • YAML validated; bash -n clean on the new logic; no eval remains.
  • Behavior preserved — flag order is irrelevant to jb, and each builder expands to the same command as before, e.g. pdflatex → jb build lectures --path-output . --builder pdflatex -n -W --keep-going.

Closes #36

🤖 Generated with Claude Code

- M10: the build command was assembled by string interpolation and run via
  `eval $BUILD_CMD`, so a space or shell metachar in source-dir/output-dir
  could break it. Invoke `jb build` directly with the directories passed via
  env and quoted, and builder-specific flags in a bash array. No eval.
- L17: default `output-dir` `./` -> `.` (drops the `.//_build` double-slash).
- L22: `extra-args` is still intentionally word-split (e.g. `-W --keep-going`);
  documented in the step that quoted args with embedded spaces aren't supported.

Behavior preserved (flag order is irrelevant to jb); verified each builder
expands to the same command.

Closes #36

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 16, 2026 05:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the build-lectures composite action’s shell invocation by removing eval, quoting path inputs, and restructuring builder selection to be safer and more predictable when paths contain spaces or shell metacharacters.

Changes:

  • Replaces interpolated eval $BUILD_CMD with a direct jb build invocation using quoted path arguments and a builder_args bash array.
  • Updates the output-dir default from ./ to . to avoid .//_build/... paths.
  • Documents (in-step) that extra-args is intentionally word-split (unquoted) to allow multiple flags.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
build-lectures/action.yml Drops eval, quotes path inputs via env vars, uses an array + case for builder flags, and adjusts output-dir default.
CHANGELOG.md Adds an Unreleased entry describing the shell-safety changes and default tweak.

Comment thread build-lectures/action.yml
Comment thread build-lectures/action.yml
Comment thread build-lectures/action.yml Outdated
- README: output-dir default ./ -> . (matches the action default).
- extra-args: document the unquoted/word-split limitation in both the
  action.yml input description and the README row (user-facing behavior).
- build-path: compute once and write via the <<EOF delimiter form (the
  repo's existing pattern) instead of four duplicated echoes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mmcky
mmcky merged commit d8ec6ca into main Jun 16, 2026
@mmcky
mmcky deleted the fix/build-lectures-shell-safety branch June 16, 2026 06:20
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.

build-lectures: shell safety (drop eval, quote inputs)

2 participants