Skip to content

rill-ci-guards: add GitHub Actions CI guards + local PII pre-push hook - #82

Merged
tarr1124 merged 3 commits into
mainfrom
feature/rill-ci-guards
Aug 10, 2026
Merged

tarr1124 merged 3 commits into
mainfrom
feature/rill-ci-guards

Conversation

@tarr1124

@tarr1124 tarr1124 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the two guard gaps behind the 2026-07-06 PII incident: no CI, and push-time scans that only saw diff content.

  • .github/workflows/ci.yml — 3 jobs on every PR to main:
    • lint: bash -n over all bin/ / plugins/ / test/ shell files (gap-free git ls-files enumeration, verified against the tree) + shellcheck pinned to v0.11.0 (sha256-verified download) at --severity=warning
    • test: every pure-shell test/cli/ suite (10 today, enumerated via glob so future suites are picked up automatically) on ubuntu + macos; no claude CLI anywhere
    • guard: CJK allowlist scan + email/phone/secrets regex scan, each run over the full tree and over the PR's commit messages
  • test/cli/cjk-guard.py (stdlib-only): tree mode filtered by test/cjk-allowlist.txt; --raw parity mode (hit-set verified identical to rg's \p{Hiragana}\p{Katakana}\p{Han} scan); --stdin for commit messages (no allowlist)
  • test/cli/pii-regex-guard.sh: generic email/phone/secret patterns only (no private vocabulary); per-value allowlisting — allowlisted matches are stripped and the remainder re-scanned, so a real secret sharing a line with a placeholder still fails
  • bin/hooks/pre-push-pii-mapping-check.sh: local-only contributor hook; reads private terms from an out-of-repo file ($RILL_DEV_PII_TERMS_FILE or ~/.config/rill-dev/pii-terms.txt), no-ops when absent, blocks on term hits in pushed diffs or commit messages (zero-SHA new-branch case via merge-base). Ships mechanism only.
  • shellcheck warning tier now clean (SC2034/SC2155 fixed in bin/rill, SC2088 disabled with reason at 2 call sites, source= directives added); .shellcheckrc documents the policy (rc-file severity is not honored by shellcheck v0.11.0 — verified — so the threshold lives on the command line)

Verification

  • bash -n: 46 files, 0 failures; shellcheck v0.11.0 --severity=warning: exit 0 over the 16-file inventory
  • CJK guard parity: --raw hit-set diff vs rg reference scan = identical (20 hits, all allowlisted)
  • PII guard: 0 unallowlisted tree hits; negative tests confirmed a same-line real address next to an allowlisted placeholder is flagged, and a rogue value inside the allowlist file itself is flagged
  • Pre-push hook dry-run in a scratch repo: no-terms-file no-op / commit-message block / new-branch (zero-SHA) content block / clean pass — 4/4
  • All 10 test/cli/ suites pass locally on macOS after the bin/rill edits (pipefail static audit included)
  • Codex review: 2 rounds of findings (phone/secret pattern coverage, allowlist self-reference loophole, per-value allowlisting) all addressed; final round: 0 findings
  • The pull_request-triggered workflow running on this PR is itself part of the verification — the ubuntu leg cannot be exercised locally

Deviations from the task Plan

  • Commit-message scans use git log base..head after a fetch-depth: 0 checkout instead of gh api (offline, no extra token scope)
  • .shellcheckrc carries policy documentation only; severity=warning moved to the CI command line (rc-file support verified absent in v0.11.0)
  • test/cjk-allowlist.txt header updated (2 lines) to declare Python-re syntax, matching the implementation

Merge is intentionally left to the repo owner (task ships up to PR creation only).

🤖 Generated with Claude Code

CI (.github/workflows/ci.yml), runs on every PR to main:
- lint: bash -n over all bin/plugins/test shell files (gap-free
  enumeration via git ls-files) + shellcheck pinned to v0.11.0
  (sha256-verified download) at severity=warning
- test: every pure-shell test/cli suite (10, enumerated dynamically via
  glob so future suites are picked up) on ubuntu + macos, no claude CLI
- guard: CJK allowlist scan + email/phone/secrets regex scan, each run
  over the full tree and over the PR's commit messages

New guards (both cwd-independent, wired into test/run-all.sh):
- test/cli/cjk-guard.py: stdlib-only; tree mode filtered by
  test/cjk-allowlist.txt, --raw parity mode (hit-set verified identical
  to rg's \p{Hiragana}\p{Katakana}\p{Han} scan), --stdin mode for
  commit messages (no allowlist)
- test/cli/pii-regex-guard.sh: generic email/phone/secret patterns only
  (no private vocabulary), filtered by new test/pii-regex-allowlist.txt
  (placeholder addresses, test fixtures, journal-timestamp filenames)

Local-only dev hook (mechanism only, never distributed to vaults):
- bin/hooks/pre-push-pii-mapping-check.sh: blocks pushes whose commits
  (added diff lines or messages) contain private terms read from an
  out-of-repo file ($RILL_DEV_PII_TERMS_FILE or
  ~/.config/rill-dev/pii-terms.txt); no-op when the file is absent;
  handles the new-branch zero-SHA case via merge-base. Header documents
  the role split against pre-commit-pii-check.sh (vault-content guard).

shellcheck adoption (warning tier now clean):
- bin/rill: drop dead SOURCES_DIR (SC2034); split declare/assign for
  target in the command-link loop (SC2155)
- plugins/{twitter,voice-memo}/requires.sh: SC2088 disable with reason
  (require_dir expands ~ itself)
- 5 plugin requires.sh: shellcheck source= directive for _lib.sh
- .shellcheckrc: policy doc; severity threshold lives on the command
  line (rc-file severity is not honored, verified on v0.11.0)
…ntu leg

The PR's own first CI run (the ubuntu leg this task could not exercise
locally) failed 4 suites: eval-distribution, portability,
codex-projection, track-managed-gitignore. Root cause, reproduced in an
ubuntu:24.04 container: 39 standalone ((var++)) sites (29 in bin/rill,
10 in plugin adapters). When the variable is 0, the arithmetic command
returns status 1; bash >= 4.1 honors set -e there and kills the script,
while macOS bash 3.2 does not -- so every counter first-increment was a
latent Linux crash. rill update died this way on ubuntu, cascading into
the 4 suite failures.

Mechanical fix, no semantic change: ((var++)) -> var=$((var + 1)),
matching the assignment-form convention test/run-all.sh already
documents. C-style for loops, awk, and jq (( usages are untouched.

Verified: all 10 test/cli suites + both tree guards pass in an
ubuntu:24.04 container as a non-root user (the checkpoint-hooks
read-only case requires non-root, same as the CI runner), and the full
macOS battery (shellcheck warning tier, bash -n, 10 suites, guards)
stays green.
Follow-up to codex review: a tracked filename starting with '-' (e.g.
'--include=x') would be parsed by grep as an option, and the previous
2>/dev/null swallowed the resulting error, so a PII-bearing file could
go unscanned while the guard still exited 0. Pass the pattern with -e
and terminate options with -- before the xargs-appended file list, and
let grep errors reach stderr. Verified with a scratch repo containing a
dash-prefixed tracked file: it is scanned and its hit reported.
@tarr1124

tarr1124 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Two follow-up commits after the PR's own first CI run (which was itself part of the verification):

  • f3cd9c5 — the ubuntu leg failed 4 suites on the first run. Root cause (reproduced in an ubuntu:24.04 container): 39 standalone ((var++)) sites; when the counter is 0 the arithmetic command returns status 1, and bash >= 4.1 honors set -e there while macOS bash 3.2 does not — so every first increment was a latent Linux crash (this is what killed rill update on ubuntu). Mechanical fix to the assignment form var=$((var + 1)) already documented in test/run-all.sh. All 10 suites + both guards verified green in the container as a non-root user, and the macOS battery stays green.
  • 1668400 — codex-review hardening: dash-prefixed tracked filenames could be parsed as grep options (with the error previously swallowed by 2>/dev/null); now passed after -e/--, with grep errors visible on stderr.

Final state: CI green on all 4 checks (lint / test ubuntu / test macos / guard), codex review clean (0 findings).

@tarr1124
tarr1124 merged commit 609815e into main Aug 10, 2026
4 checks passed
@tarr1124
tarr1124 deleted the feature/rill-ci-guards branch August 10, 2026 05:49
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.

1 participant