Files
ironclaw/scripts/pre-commit-safety.sh
Illia Polosukhin 8b6298513d feat(i18n): add Korean translation, fix zh-CN drift, and prevent future drift via pre-commit hook (#2065)
* feat(i18n): add Korean translation, fix zh-CN drift, cover hardcoded strings

Adds Korean (ko) as the third web UI language, brings zh-CN back into
parity with en, converts ~80 hardcoded English strings in app.js into
i18n keys, and installs a pre-commit hook that prevents future drift.

## Korean web UI

- New `src/channels/web/static/i18n/ko.js` — full translation of all
  663 keys, mirroring the structure of `en.js`/`zh-CN.js`
- New `src/channels/web/server.rs` route `/i18n/ko.js` + handler
- New language menu button in `index.html`
- Browser auto-detect now special-cases `ko-*` (in addition to `zh-*`)
  so Korean visitors land on Korean by default
- Toast label map in `i18n-app.js` becomes a small lookup table so the
  next language is a single-line addition

## zh-CN drift fix

`zh-CN.js` was missing 9 keys that had been added to `en.js` after the
Chinese pack was last touched (`config.telegramOpenBot`,
`settings.tools`, and 7 keys under the `tools.*` namespace for the new
Tool Permissions tab). Backfilled with Chinese translations so users on
the Tools settings panel see proper labels instead of raw key strings.

## Hardcoded strings in app.js

`app.js` had ~80 user-facing English string literals that bypassed
`I18n.t()` entirely — toasts, confirms, alerts, button labels, meta-item
labels for jobs/routines/missions detail panels, the theme dynamic
label, dynamic auth states ("Connecting...", "Authenticated"), etc.
These were invisible to the language switcher and would always render
in English regardless of the user's choice.

Replaced every literal with `I18n.t('key', { ...placeholders })` and
added the corresponding ~95 new keys to `en.js`, `zh-CN.js`, AND `ko.js`
in lockstep so all three packs stay at 663 keys with identical key sets
and matching `{name}`-style placeholder tokens.

Existing keys were reused where possible (`message.copy`,
`approval.approved`, `connection.reconnected`, etc.).

## Pre-commit parity hook

New `scripts/check-i18n-parity.sh` (pure POSIX bash, no Node) verifies:

1. No duplicate keys within any single language file
2. Every language has the same key set as `en.js` (the source of truth)
3. Placeholder tokens like `{name}`, `{count}` match across all
   languages — catches the silent bug where a translator drops an
   interpolation token

Wired into both pre-commit hook install paths:
- `scripts/pre-commit-safety.sh` (installed by `dev-setup.sh` as a
  symlink at `.git/hooks/pre-commit`; symlink is followed via
  `readlink` so the script location resolves correctly)
- `.githooks/pre-commit` (used when devs set
  `git config core.hooksPath .githooks`)

Both block the commit on failure with a clear error message and the
`git commit --no-verify` escape hatch. Tested by deliberately removing
a key from `ko.js` (caught) and stripping a `{path}` placeholder
(caught).

## Korean README

New `README.ko.md` — full Korean translation of `README.md`. Follows
the layout of `README.ja.md` (6-item single-word ToC to keep anchors
clean for non-Latin headings). All code blocks, image paths, and badge
URLs preserved verbatim.

`한국어` link added to the language switcher in all 5 READMEs
(`README.md`, `.zh-CN.md`, `.ru.md`, `.ja.md`, and the new `.ko.md`).

## Verification

- `./scripts/check-i18n-parity.sh` — `OK (663 keys × 3 languages)`
- `node --check` clean on every modified JS file
- Three-way parity: identical sorted key sets across en/zh-CN/ko, zero
  placeholder mismatches
- Hook tested by removing/mutating keys and confirming the commit is
  blocked

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

* fix(i18n): address PR review feedback [skip-regression-check]

Addresses 6 review comments on #2065. All changes are in
src/channels/web/static/ (per .claude/rules/review-discipline.md
exemption) plus a bash helper script — no Rust code is touched.

## scripts/check-i18n-parity.sh

- **Portable mktemp** (Copilot): bare `mktemp` works on GNU but BSD/macOS
  `mktemp` requires an explicit template with at least 6 trailing X's.
  Wrap in a small `mktemp_file()` helper that always passes a template
  (`${TMPDIR:-/tmp}/check-i18n-parity.XXXXXX`) so the script runs on
  every platform.

- **Symlink-attack-prone /tmp path** (gemini-code-assist): the
  placeholder-mismatch buffer was using `/tmp/i18n-ph-mismatch.$$`,
  which is predictable and vulnerable to symlink races in shared
  /tmp. Replace with `mktemp_file()` for consistency with the rest
  of the script.

## src/channels/web/static/app.js

- **Hardcoded `'Mode'` label** (gemini): jobs detail meta-grid had
  `metaItem('Mode', job.job_mode)` — convert to
  `I18n.t('jobs.mode')` and add the new key to all 3 language packs.

- **Hardcoded `'Yes'`/`'No'`** (Copilot): routine detail showed
  `routine.enabled ? 'Yes' : 'No'` even though the surrounding labels
  were translated. Reuse the existing `settings.on`/`settings.off`
  keys ("On"/"Off") which already render in all languages.

- **Hardcoded `'N/A'`** (Copilot): mission detail showed
  `m.next_fire_at ? formatDate(...) : 'N/A'`. Reuse the existing
  `common.noData` key. Also fixed the same pattern in the TEE popover
  (`renderTeePopover`) where `'N/A'` was used as a fallback for
  three different attestation fields, since fixing the pattern
  across the file is the principled response per the repo's
  review-discipline rule.

## src/channels/web/static/i18n-app.js

- **Hardcoded `LANG_LABELS` map** (gemini): the language-switch toast
  was reading from a per-call `{ 'en': 'English', 'zh-CN': '简体中文',
  'ko': '한국어' }` literal that would grow with every new language
  and drift from the actual supported set. Move each language's own
  native name into its own pack under a new `language.name` key:

      en.js    → 'language.name': 'English'
      zh-CN.js → 'language.name': '简体中文'
      ko.js    → 'language.name': '한국어'

  Then the toast becomes `I18n.t('language.switch') + ': ' +
  I18n.t('language.name')` — both halves are read from the language
  pack that was just switched in, so the entire toast appears in the
  newly selected language. Adding a future language is now a single
  key addition with NO changes to i18n-app.js.

## Verification

  $ ./scripts/check-i18n-parity.sh
  i18n parity: OK (665 keys × 3 languages)

  $ cargo test --lib
  test result: ok. 4241 passed; 0 failed; 3 ignored

Three-way parity preserved with the 2 new keys (`jobs.mode` and
`language.name`) added to all three language packs in lockstep.

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

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 09:22:19 -07:00

198 lines
8.6 KiB
Bash
Executable File

#!/usr/bin/env bash
# Pre-commit safety checks for common issues caught by AI code reviewers.
#
# Can be run standalone: bash scripts/pre-commit-safety.sh
# Or installed as a git pre-commit hook via dev-setup.sh.
#
# Checks staged .rs files for:
# 1. Unsafe UTF-8 byte slicing (panics on multi-byte chars)
# 2. Case-sensitive file extension comparisons
# 3. Hardcoded /tmp paths in tests (flaky in parallel runs)
# 4. Tool parameters logged without redaction (secret leaks)
# 5. Multi-step DB operations without transaction wrapping
# 6. .unwrap(), .expect(), assert!() in production code (panics)
#
# Also runs check-i18n-parity.sh when src/channels/web/static/i18n/*.js
# files are staged, to ensure every language pack has the same key set.
#
# Suppress individual lines with an inline "// safety: <reason>" comment.
set -euo pipefail
# Determine a suitable base ref for standalone diffs.
resolve_base_ref() {
local candidates=(
"@{upstream}"
"origin/HEAD"
"origin/main"
"origin/master"
"main"
"master"
)
for ref in "${candidates[@]}"; do
if git rev-parse --verify --quiet "$ref" >/dev/null 2>&1; then
echo "$ref"
return 0
fi
done
echo "pre-commit-safety: could not determine a base Git ref for diff (tried: ${candidates[*]})." >&2
echo "pre-commit-safety: ensure your repository has an upstream or a local main/master branch." >&2
exit 1
}
# i18n parity: when any language pack changes, all languages must stay in sync.
# Run before the .rs-focused checks so it fires even when no .rs files change.
if git diff --cached --quiet 2>/dev/null; then
I18N_CHANGED=$(git diff --name-only -- 'src/channels/web/static/i18n/*.js' 2>/dev/null || true)
else
I18N_CHANGED=$(git diff --cached --name-only -- 'src/channels/web/static/i18n/*.js' 2>/dev/null || true)
fi
if [ -n "$I18N_CHANGED" ]; then
# Resolve script location even when invoked via a symlink (the
# pre-commit hook is typically a symlink at .git/hooks/pre-commit
# pointing to this file in scripts/). Walk symlinks until we find the
# real path, then use its parent directory.
SOURCE="${BASH_SOURCE[0]:-$0}"
while [ -L "$SOURCE" ]; do
LINK_TARGET="$(readlink "$SOURCE")"
case "$LINK_TARGET" in
/*) SOURCE="$LINK_TARGET" ;;
*) SOURCE="$(cd "$(dirname "$SOURCE")" && pwd)/$LINK_TARGET" ;;
esac
done
SCRIPT_DIR="$(cd "$(dirname "$SOURCE")" && pwd)"
if ! "$SCRIPT_DIR/check-i18n-parity.sh"; then
echo ""
echo "Commit blocked: i18n parity check failed."
echo "Every key added to en.js must also be added to all other language files (zh-CN.js, ko.js, ...)."
echo "Placeholder tokens like {name} must match across all languages."
echo "To bypass: git commit --no-verify"
exit 1
fi
fi
# Support both pre-commit hook (staged files) and standalone (all changed vs base)
if git diff --cached --quiet 2>/dev/null; then
# No staged changes -- compare working tree against a resolved base ref
BASE_REF="$(resolve_base_ref)"
DIFF_OUTPUT=$(git diff "$BASE_REF" -- '*.rs' 2>/dev/null || true)
else
DIFF_OUTPUT=$(git diff --cached -U0 -- '*.rs' 2>/dev/null || true)
fi
# Early exit if there are no relevant .rs changes
if [ -z "$DIFF_OUTPUT" ]; then
exit 0
fi
WARNINGS=0
warn() {
if [ "$WARNINGS" -eq 0 ]; then
echo ""
echo "=== Pre-commit Safety Checks ==="
echo ""
fi
WARNINGS=$((WARNINGS + 1))
echo " [$1] $2"
}
# 1. Unsafe UTF-8 byte slicing: &s[..N] or &s[..some_var] on strings
# Safe patterns: is_char_boundary, char_indices, // safety:
if echo "$DIFF_OUTPUT" | grep -nE '^\+' | grep -E '\[\.\..*\]' | grep -vE 'is_char_boundary|char_indices|// safety:|as_bytes|Vec<|&\[u8\]|\[u8\]|bytes\(\)|&bytes' | head -3 | grep -q .; then
warn "UTF8" "Possible unsafe byte-index string slicing. Use is_char_boundary() or char_indices()."
echo "$DIFF_OUTPUT" | grep -nE '^\+' | grep -E '\[\.\..*\]' | grep -vE 'is_char_boundary|char_indices|// safety:|as_bytes|Vec<|&\[u8\]|\[u8\]|bytes\(\)|&bytes' | head -3 | sed 's/^/ /'
fi
# 2. Case-sensitive file extension checks
# Match: .ends_with(".png") without prior to_lowercase
if echo "$DIFF_OUTPUT" | grep -nE '^\+.*ends_with\("\.([pP][nN][gG]|[jJ][pP][eE]?[gG]|[gG][iI][fF]|[wW][eE][bB][pP]|[mM][dD])"\)' | grep -vE 'to_lowercase|to_ascii_lowercase|// safety:' | head -3 | grep -q .; then
warn "CASE" "Case-sensitive file extension comparison. Normalize to lowercase first."
echo "$DIFF_OUTPUT" | grep -nE '^\+.*ends_with\("\.([pP][nN][gG]|[jJ][pP][eE]?[gG]|[gG][iI][fF]|[wW][eE][bB][pP]|[mM][dD])"\)' | grep -vE 'to_lowercase|to_ascii_lowercase|// safety:' | head -3 | sed 's/^/ /'
fi
# 3. Hardcoded /tmp paths in test files
if echo "$DIFF_OUTPUT" | grep -nE '^\+.*"/tmp/' | grep -vE 'tempfile|tempdir|// safety:' | head -3 | grep -q .; then
warn "TMPDIR" "Hardcoded /tmp path. Use tempfile::tempdir() for parallel-safe tests."
echo "$DIFF_OUTPUT" | grep -nE '^\+.*"/tmp/' | grep -vE 'tempfile|tempdir|// safety:' | head -3 | sed 's/^/ /'
fi
# 4. Logging tool parameters without redaction
if echo "$DIFF_OUTPUT" | grep -nE '^\+.*tracing::(info|debug|warn|error).*param' | grep -vE 'redact|// safety:' | head -3 | grep -q .; then
warn "REDACT" "Logging tool parameters without redaction. Use redact_params() first."
echo "$DIFF_OUTPUT" | grep -nE '^\+.*tracing::(info|debug|warn|error).*param' | grep -vE 'redact|// safety:' | head -3 | sed 's/^/ /'
fi
# 5. Multi-step DB operations without transaction
# Uses -W (function context) to reduce false positives from existing transactions.
# Suppressible with "// safety:" in the hunk.
DIFF_W_OUTPUT=$(git diff --cached -W -- '*.rs' 2>/dev/null || git diff "$(resolve_base_ref)" -W -- '*.rs' 2>/dev/null || true)
if [ -n "$DIFF_W_OUTPUT" ]; then
HUNK_COUNT=$(echo "$DIFF_W_OUTPUT" | awk '
/^@@/ {
if (count >= 2 && !has_tx && !has_safety) found++
count=0; has_tx=0; has_safety=0
}
/^\+.*\.(execute|query)\(/ { count++ }
/^\+.*(transaction|\.tx\.|\.begin\()/ { has_tx=1 }
/ .*(transaction|\.tx\.|\.begin\()/ { has_tx=1 }
/\/\/ safety:/ { has_safety=1 }
END {
if (count >= 2 && !has_tx && !has_safety) found++
print found+0
}
')
if [ "$HUNK_COUNT" -gt 0 ]; then
warn "TX" "Multiple DB operations in same function without transaction. Wrap in a transaction for atomicity."
echo "$DIFF_W_OUTPUT" | awk '
/^@@/ {
if (count >= 2 && !has_tx && !has_safety) { print buf }
buf=""; count=0; has_tx=0; has_safety=0
}
/^\+.*\.(execute|query)\(/ { count++ }
/^\+.*(transaction|\.tx\.|\.begin\()/ { has_tx=1 }
/ .*(transaction|\.tx\.|\.begin\()/ { has_tx=1 }
/\/\/ safety:/ { has_safety=1 }
{ buf = buf "\n" $0 }
END {
if (count >= 2 && !has_tx && !has_safety) { print buf }
}
' | grep -E '^\+.*\.(execute|query)\(' | head -4 | sed 's/^/ /'
fi
fi
# 6. .unwrap(), .expect(), assert!() in production code
# Matches added lines containing panic-inducing calls.
# Excludes test files, test modules, and debug_assert (compiled out in release).
# Suppress with "// safety: <reason>".
PROD_DIFF="$DIFF_OUTPUT"
# Strip hunks from test-only files (tests/ directory, *_test.rs, test_*.rs)
PROD_DIFF=$(echo "$PROD_DIFF" | grep -v '^+++ b/tests/' || true)
# Strip hunks whose @@ context line indicates a test module.
# git diff includes the enclosing function/module name after @@.
# Only match `mod tests` (the conventional #[cfg(test)] module) — do NOT
# match `fn test_*` because production code can have functions named test_*.
PROD_DIFF=$(echo "$PROD_DIFF" | awk '
/^@@ / { in_test = ($0 ~ /mod tests/) }
!in_test { print }
' || true)
if echo "$PROD_DIFF" | grep -nE '^\+' \
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
| grep -vE 'debug_assert|// safety:|#\[cfg\(test\)\]|#\[test\]|mod tests' \
| head -5 | grep -q .; then
warn "PANIC" "Production code must not use .unwrap(), .expect(), or assert!(). Use proper error handling."
echo "$PROD_DIFF" | grep -nE '^\+' \
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
| grep -vE 'debug_assert|// safety:|#\[cfg\(test\)\]|#\[test\]|mod tests' \
| head -5 | sed 's/^/ /'
fi
if [ "$WARNINGS" -gt 0 ]; then
echo ""
echo "Found $WARNINGS potential issue(s). Fix them or add '// safety: <reason>' to suppress."
echo ""
exit 1
fi