From 680aa4f43c9b6fb0b1d546f7268bb3dcaae241ca Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Sun, 9 Aug 2026 20:14:23 +0000 Subject: [PATCH] refactor(kyberforge): consolidate vale-wrap.sh's config parsing and subprocess spawns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two efficiency findings from a code-review pass: - The separated (--config X) and joined (--config=X) argument branches duplicated ~20 lines of absolutize-if-relative path logic. Extracted into abs_config_value(), used by both branches; the redundant --config=/* special case falls out since the helper already passes absolute paths through unchanged. - The common single-file path spawned python3 twice per file (once for abspath resolution, once for flatten()). flatten() now optionally takes a tmpdir arg and does both in one process. The per-file loop under a directory argument is unchanged — that path wasn't flagged. agent-audit's copy is canonical; skill-audit's copy was regenerated via scripts/sync-vale-styles.sh, not hand-edited, to guarantee byte parity. No hardening (bash 3.2 compat, surrogateescape, symlink guards) touched. Verified: tests/test-vale-wrap.sh 39/39, check-vale-style-sync.sh clean, full suite 12/12. --- .../skills/agent-audit/scripts/vale-wrap.sh | 85 +++++++++++++------ .../skills/skill-audit/scripts/vale-wrap.sh | 85 +++++++++++++------ 2 files changed, 114 insertions(+), 56 deletions(-) diff --git a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh index dae60b4..862f44c 100755 --- a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh @@ -77,6 +77,17 @@ is_builtin_output() { *) false ;; esac } +# Absolutizes a `--config` value against the caller's cwd. Shared by both +# argument forms below — separated (`--config X`) and joined (`--config=X`) +# — so the "already absolute vs. needs $cwd prefixed" check lives in exactly +# one place instead of being duplicated per form. +abs_config_value() { + if [[ "$1" == /* ]]; then + printf '%s' "$1" + else + printf '%s' "$cwd/$1" + fi +} for arg in "$@"; do if [[ -n "$pending_flag" ]]; then # Value of a separated two-argv flag. It is never a lint target, however @@ -85,11 +96,7 @@ for arg in "$@"; do case "$pending_flag" in --config) # Always a path, and required to exist. - if [[ "$arg" == /* ]]; then - vale_args+=("$arg") - else - vale_args+=("$cwd/$arg") - fi + vale_args+=("$(abs_config_value "$arg")") ;; --output|--path) # See `is_builtin_output` above for why the built-in `--output` names @@ -117,13 +124,8 @@ for arg in "$@"; do config_given=true continue ;; - --config=/*) - vale_args+=("$arg") - config_given=true - continue - ;; --config=*) - vale_args+=("--config=$cwd/${arg#--config=}") + vale_args+=("--config=$(abs_config_value "${arg#--config=}")") config_given=true continue ;; @@ -204,11 +206,33 @@ abspath() { } flatten() { - python3 - "$1" "$2" <<'PYTHON' + # Two call shapes: `flatten src dest` (dest already resolved and inside the + # scratch tree — the per-markdown-file calls in the directory branch below) + # writes straight to `dest`. `flatten src raw_dest tmpdir` (the single-file + # branch further down) additionally resolves `raw_dest` the way a separate + # `abspath` call used to, applies the same sandbox-escape guard, and prints + # the resolved path — folding two python3 spawns per file into one. + python3 - "$@" <<'PYTHON' +import os import re import sys -src, dest = sys.argv[1], sys.argv[2] +src, dest_input = sys.argv[1], sys.argv[2] +tmpdir = sys.argv[3] if len(sys.argv) > 3 else None + +if tmpdir is None: + dest = dest_input +else: + dest = os.path.abspath(dest_input) + if not dest.startswith(tmpdir + os.sep): + print( + f"vale-wrap.sh: refusing to lint '{src}': its scratch copy would " + f"land outside {tmpdir}", + file=sys.stderr, + ) + sys.exit(2) + os.makedirs(os.path.dirname(dest), exist_ok=True) + # surrogateescape keeps a non-UTF-8 file (reachable via a directory argument) # a byte-for-byte round trip instead of aborting the whole run on a decode error. with open(src, encoding='utf-8', errors='surrogateescape') as fh: @@ -435,6 +459,9 @@ if header_m: with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh: fh.write(content) + +if tmpdir is not None: + print(dest) PYTHON } @@ -449,23 +476,23 @@ mkdir -p "$mirror" argv_paths=() for arg in ${path_args[@]+"${path_args[@]}"}; do if [[ "$arg" == /* ]]; then - dest="$tmpdir$arg" + raw_dest="$tmpdir$arg" else - dest="$mirror/$arg" + raw_dest="$mirror/$arg" fi - dest="$(abspath "$dest")" - # A path argument with enough leading `..` to climb past the mirror root would - # write outside the scratch dir. The real filesystem clamps such a path at - # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. - case "$dest" in - "$tmpdir"/*) ;; - *) - echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 - exit 2 - ;; - esac - mkdir -p "$(dirname "$dest")" if [[ -d "$arg" ]]; then + dest="$(abspath "$raw_dest")" + # A path argument with enough leading `..` to climb past the mirror root would + # write outside the scratch dir. The real filesystem clamps such a path at + # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. + case "$dest" in + "$tmpdir"/*) ;; + *) + echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 + exit 2 + ;; + esac + mkdir -p "$(dirname "$dest")" # A directory is mirrored whole — vale applies its own format filtering to # the tree, so any file dropped here would be silently unlinted — and then # every markdown file in the copy is flattened in place. `.git` is pruned: @@ -484,7 +511,9 @@ for arg in ${path_args[@]+"${path_args[@]}"}; do flatten "$md" "$md" done < <(find "$dest" -type f -name '*.md' -print0) else - flatten "$arg" "$dest" + # `abspath` + `flatten` folded into one python3 process — see the comment + # atop `flatten` above. + dest="$(flatten "$arg" "$raw_dest" "$tmpdir")" fi if [[ "$arg" == /* ]]; then argv_paths+=("$dest") diff --git a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh index dae60b4..862f44c 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh @@ -77,6 +77,17 @@ is_builtin_output() { *) false ;; esac } +# Absolutizes a `--config` value against the caller's cwd. Shared by both +# argument forms below — separated (`--config X`) and joined (`--config=X`) +# — so the "already absolute vs. needs $cwd prefixed" check lives in exactly +# one place instead of being duplicated per form. +abs_config_value() { + if [[ "$1" == /* ]]; then + printf '%s' "$1" + else + printf '%s' "$cwd/$1" + fi +} for arg in "$@"; do if [[ -n "$pending_flag" ]]; then # Value of a separated two-argv flag. It is never a lint target, however @@ -85,11 +96,7 @@ for arg in "$@"; do case "$pending_flag" in --config) # Always a path, and required to exist. - if [[ "$arg" == /* ]]; then - vale_args+=("$arg") - else - vale_args+=("$cwd/$arg") - fi + vale_args+=("$(abs_config_value "$arg")") ;; --output|--path) # See `is_builtin_output` above for why the built-in `--output` names @@ -117,13 +124,8 @@ for arg in "$@"; do config_given=true continue ;; - --config=/*) - vale_args+=("$arg") - config_given=true - continue - ;; --config=*) - vale_args+=("--config=$cwd/${arg#--config=}") + vale_args+=("--config=$(abs_config_value "${arg#--config=}")") config_given=true continue ;; @@ -204,11 +206,33 @@ abspath() { } flatten() { - python3 - "$1" "$2" <<'PYTHON' + # Two call shapes: `flatten src dest` (dest already resolved and inside the + # scratch tree — the per-markdown-file calls in the directory branch below) + # writes straight to `dest`. `flatten src raw_dest tmpdir` (the single-file + # branch further down) additionally resolves `raw_dest` the way a separate + # `abspath` call used to, applies the same sandbox-escape guard, and prints + # the resolved path — folding two python3 spawns per file into one. + python3 - "$@" <<'PYTHON' +import os import re import sys -src, dest = sys.argv[1], sys.argv[2] +src, dest_input = sys.argv[1], sys.argv[2] +tmpdir = sys.argv[3] if len(sys.argv) > 3 else None + +if tmpdir is None: + dest = dest_input +else: + dest = os.path.abspath(dest_input) + if not dest.startswith(tmpdir + os.sep): + print( + f"vale-wrap.sh: refusing to lint '{src}': its scratch copy would " + f"land outside {tmpdir}", + file=sys.stderr, + ) + sys.exit(2) + os.makedirs(os.path.dirname(dest), exist_ok=True) + # surrogateescape keeps a non-UTF-8 file (reachable via a directory argument) # a byte-for-byte round trip instead of aborting the whole run on a decode error. with open(src, encoding='utf-8', errors='surrogateescape') as fh: @@ -435,6 +459,9 @@ if header_m: with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh: fh.write(content) + +if tmpdir is not None: + print(dest) PYTHON } @@ -449,23 +476,23 @@ mkdir -p "$mirror" argv_paths=() for arg in ${path_args[@]+"${path_args[@]}"}; do if [[ "$arg" == /* ]]; then - dest="$tmpdir$arg" + raw_dest="$tmpdir$arg" else - dest="$mirror/$arg" + raw_dest="$mirror/$arg" fi - dest="$(abspath "$dest")" - # A path argument with enough leading `..` to climb past the mirror root would - # write outside the scratch dir. The real filesystem clamps such a path at - # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. - case "$dest" in - "$tmpdir"/*) ;; - *) - echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 - exit 2 - ;; - esac - mkdir -p "$(dirname "$dest")" if [[ -d "$arg" ]]; then + dest="$(abspath "$raw_dest")" + # A path argument with enough leading `..` to climb past the mirror root would + # write outside the scratch dir. The real filesystem clamps such a path at + # `/`; the mirror can't, so refuse rather than scribble outside the sandbox. + case "$dest" in + "$tmpdir"/*) ;; + *) + echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2 + exit 2 + ;; + esac + mkdir -p "$(dirname "$dest")" # A directory is mirrored whole — vale applies its own format filtering to # the tree, so any file dropped here would be silently unlinted — and then # every markdown file in the copy is flattened in place. `.git` is pruned: @@ -484,7 +511,9 @@ for arg in ${path_args[@]+"${path_args[@]}"}; do flatten "$md" "$md" done < <(find "$dest" -type f -name '*.md' -print0) else - flatten "$arg" "$dest" + # `abspath` + `flatten` folded into one python3 process — see the comment + # atop `flatten` above. + dest="$(flatten "$arg" "$raw_dest" "$tmpdir")" fi if [[ "$arg" == /* ]]; then argv_paths+=("$dest")