feat(M4): Reusable code abstractions and dead-code removal
This commit is contained in:
@@ -0,0 +1,77 @@
|
||||
/**
|
||||
* safe-replace-dir.mjs — safely replace a directory within a safety-root boundary
|
||||
*
|
||||
* Exports:
|
||||
* safeReplaceDir(source, target, safetyRoot) → Promise<void>
|
||||
*
|
||||
* Usage:
|
||||
* import { safeReplaceDir } from "./lib/safe-replace-dir.mjs";
|
||||
* await safeReplaceDir("/path/to/source", "/safe/root/target", "/safe/root");
|
||||
*
|
||||
* Safety contract:
|
||||
* - `target` must be a strict descendant of `safetyRoot` (not equal to it).
|
||||
* - `target` must be a non-empty path.
|
||||
* - Throws with a descriptive message if either constraint is violated.
|
||||
*
|
||||
* Behaviour:
|
||||
* - Removes any existing content at `target` (rm -rf equivalent).
|
||||
* - Creates `target` (and any missing parent directories).
|
||||
* - Copies all files from `source` into `target`.
|
||||
*/
|
||||
|
||||
import { cp, mkdir, realpath, rm } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
|
||||
/**
|
||||
* Safely replace `target` with the contents of `source`, enforcing that
|
||||
* `target` is a strict descendant of `safetyRoot`.
|
||||
*
|
||||
* @param {string} source - Directory to copy from.
|
||||
* @param {string} target - Directory to replace (will be removed then recreated).
|
||||
* @param {string} safetyRoot - Ancestor boundary; `target` must be inside this.
|
||||
* @returns {Promise<void>}
|
||||
*/
|
||||
export async function safeReplaceDir(source, target, safetyRoot) {
|
||||
if (!target || target === "") {
|
||||
throw new Error(`Refusing to replace unsafe target: (empty string)`);
|
||||
}
|
||||
|
||||
const resolvedSafety = path.resolve(safetyRoot);
|
||||
const resolvedTarget = path.resolve(target);
|
||||
|
||||
// Lexical check: target must be a strict descendant of safetyRoot.
|
||||
const relative = path.relative(resolvedSafety, resolvedTarget);
|
||||
if (!relative || relative.startsWith("..") || path.isAbsolute(relative) || relative === "") {
|
||||
throw new Error(`Refusing to replace target outside safety root: ${target}`);
|
||||
}
|
||||
|
||||
// Real-path check: resolve the deepest existing ancestor of target's parent
|
||||
// and verify it lies inside the real (symlink-resolved) safety root.
|
||||
// This blocks a symlinked parent directory from redirecting outside the boundary.
|
||||
const realSafety = await realpath(resolvedSafety);
|
||||
let checkPath = path.dirname(resolvedTarget);
|
||||
for (;;) {
|
||||
try {
|
||||
const realAncestor = await realpath(checkPath);
|
||||
const realRel = path.relative(realSafety, realAncestor);
|
||||
if (realRel.startsWith("..") || path.isAbsolute(realRel)) {
|
||||
throw new Error(`Refusing to replace target outside safety root: ${target}`);
|
||||
}
|
||||
break; // validation passed
|
||||
} catch (err) {
|
||||
if (err.code === "ENOENT") {
|
||||
const parent = path.dirname(checkPath);
|
||||
if (parent === checkPath) {
|
||||
throw new Error(`Refusing to replace target outside safety root: ${target}`, { cause: err });
|
||||
}
|
||||
checkPath = parent;
|
||||
continue;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
await rm(resolvedTarget, { recursive: true, force: true });
|
||||
await mkdir(resolvedTarget, { recursive: true });
|
||||
await cp(source, resolvedTarget, { recursive: true, force: true });
|
||||
}
|
||||
Executable
+102
@@ -0,0 +1,102 @@
|
||||
#!/usr/bin/env bash
|
||||
# safe-replace-dir.sh — safely replace a directory within a safety-root boundary
|
||||
#
|
||||
# Provides safe_replace_dir() for sourcing, or run standalone:
|
||||
# ./scripts/lib/safe-replace-dir.sh <source> <target> <safety_root>
|
||||
#
|
||||
# Safety contract (mirrors safe-replace-dir.mjs):
|
||||
# - <target> must be a non-empty path.
|
||||
# - <target> must be a strict descendant of <safety_root> (not equal to it).
|
||||
# - Prints an error and returns/exits 1 if either constraint is violated.
|
||||
#
|
||||
# Usage (sourced):
|
||||
# source "$(dirname "${BASH_SOURCE[0]}")/safe-replace-dir.sh"
|
||||
# safe_replace_dir "$source" "$target" "$safety_root"
|
||||
#
|
||||
# Usage (standalone):
|
||||
# ./scripts/lib/safe-replace-dir.sh /path/to/source /safe/root/target /safe/root
|
||||
|
||||
safe_replace_dir() {
|
||||
local source=$1
|
||||
local target=$2
|
||||
local safety_root=$3
|
||||
|
||||
if [[ -z "$target" ]]; then
|
||||
echo "safe_replace_dir: refusing to replace unsafe target: (empty string)" >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Resolve the real (symlink-resolved) safety root.
|
||||
local abs_safety
|
||||
abs_safety=$(cd "$safety_root" 2>/dev/null && pwd -P) || {
|
||||
echo "safe_replace_dir: safety root does not exist: $safety_root" >&2
|
||||
return 1
|
||||
}
|
||||
|
||||
# Build an absolute lexical path for target's parent directory.
|
||||
local target_parent target_base
|
||||
target_base=$(basename "$target")
|
||||
target_parent=$(dirname "$target")
|
||||
# Make target_parent absolute without relying on cd (target may not exist yet).
|
||||
if [[ "$target_parent" != /* ]]; then
|
||||
target_parent="${PWD}/${target_parent}"
|
||||
fi
|
||||
|
||||
# Walk up from target_parent to find the deepest existing directory,
|
||||
# accumulating the non-existing path suffix as we go.
|
||||
local suffix=""
|
||||
local walk="$target_parent"
|
||||
while [[ ! -d "$walk" ]]; do
|
||||
local component
|
||||
component=$(basename "$walk")
|
||||
if [[ -z "$suffix" ]]; then
|
||||
suffix="$component"
|
||||
else
|
||||
suffix="${component}/${suffix}"
|
||||
fi
|
||||
local next
|
||||
next=$(dirname "$walk")
|
||||
if [[ "$next" == "$walk" ]]; then
|
||||
echo "safe_replace_dir: could not find existing ancestor for: $target" >&2
|
||||
return 1
|
||||
fi
|
||||
walk="$next"
|
||||
done
|
||||
|
||||
# Resolve the real path of the existing ancestor (follows symlinks).
|
||||
local abs_parent
|
||||
abs_parent=$(cd "$walk" && pwd -P) || {
|
||||
echo "safe_replace_dir: could not resolve parent directory: $walk" >&2
|
||||
return 1
|
||||
}
|
||||
|
||||
# Reconstruct the full absolute target path.
|
||||
local abs_target
|
||||
if [[ -n "$suffix" ]]; then
|
||||
abs_target="${abs_parent}/${suffix}/${target_base}"
|
||||
else
|
||||
abs_target="${abs_parent}/${target_base}"
|
||||
fi
|
||||
|
||||
# Check that abs_target is strictly inside abs_safety
|
||||
case "$abs_target" in
|
||||
"${abs_safety}/"*) ;;
|
||||
*)
|
||||
echo "safe_replace_dir: refusing to replace target outside safety root: $target" >&2
|
||||
return 1
|
||||
;;
|
||||
esac
|
||||
|
||||
rm -rf "$abs_target"
|
||||
mkdir -p "$abs_target"
|
||||
cp -R "${source}/." "$abs_target/"
|
||||
}
|
||||
|
||||
# Allow standalone use
|
||||
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
|
||||
if [[ $# -ne 3 ]]; then
|
||||
echo "Usage: $0 <source> <target> <safety_root>" >&2
|
||||
exit 1
|
||||
fi
|
||||
safe_replace_dir "$1" "$2" "$3" || exit 1
|
||||
fi
|
||||
@@ -532,6 +532,24 @@ export async function buildOperationPlan({ selections, repoRoot = process.cwd(),
|
||||
return { operations, prompts, reportRows, assumeYes };
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove the target of an operation (skill, helper, or superpowers).
|
||||
*
|
||||
* Validates that the target is within the skills root before removing.
|
||||
* Handles both regular directories and symbolic links.
|
||||
* Idempotent: succeeds even when the target does not exist.
|
||||
*
|
||||
* @param {object} op - Operation object with at least `target` and `skillsRoot`.
|
||||
* @returns {Promise<object>} Operation with `status: "ok"`.
|
||||
*/
|
||||
export async function removeTarget(op) {
|
||||
await validateRemoveTarget(op.target, op.skillsRoot, { repoRoot: REPO_ROOT });
|
||||
const info = existsSync(op.target) ? await lstat(op.target) : null;
|
||||
if (info?.isSymbolicLink()) await unlink(op.target);
|
||||
else await rm(op.target, { recursive: true, force: true });
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
|
||||
export async function validateRemoveTarget(target, skillsRoot, { repoRoot = process.cwd() } = {}) {
|
||||
const resolvedRoot = path.resolve(skillsRoot);
|
||||
const resolvedTarget = path.resolve(target);
|
||||
@@ -599,8 +617,6 @@ export async function executeOperation(op) {
|
||||
if (op.kind === "package-skill") return { ...op, status: "included" };
|
||||
if (op.kind === "sync-pi-package") {
|
||||
// Use the canonical generator (pnpm run sync:pi / node scripts/generate-skills.mjs).
|
||||
// The legacy sync-pi-package-skills.sh is retired in M3; it bypassed the
|
||||
// generator and copied skills/*/pi into pi-package directly, corrupting manifests.
|
||||
runCommand(process.execPath, [path.join(op.repoRoot, "scripts", "generate-skills.mjs")], { cwd: op.repoRoot });
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
@@ -610,33 +626,19 @@ export async function executeOperation(op) {
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
if (op.kind === "skill") {
|
||||
if (op.action === "remove") {
|
||||
await validateRemoveTarget(op.target, op.skillsRoot, { repoRoot: REPO_ROOT });
|
||||
const info = existsSync(op.target) ? await lstat(op.target) : null;
|
||||
if (info?.isSymbolicLink()) await unlink(op.target);
|
||||
else await rm(op.target, { recursive: true, force: true });
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
if (op.action === "remove") return removeTarget(op);
|
||||
await copyDirectoryReplacing(op.source, op.target);
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
if (op.kind === "helper") {
|
||||
if (op.action === "remove") {
|
||||
await validateRemoveTarget(op.target, op.skillsRoot, { repoRoot: REPO_ROOT });
|
||||
const info = existsSync(op.target) ? await lstat(op.target) : null;
|
||||
if (info?.isSymbolicLink()) await unlink(op.target);
|
||||
else await rm(op.target, { recursive: true, force: true });
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
if (op.action === "remove") return removeTarget(op);
|
||||
await installHelperAllowlist(op);
|
||||
return { ...op, status: "ok" };
|
||||
}
|
||||
if (op.kind === "superpowers") {
|
||||
if (op.action === "remove") return removeTarget(op);
|
||||
await mkdir(path.dirname(op.target), { recursive: true });
|
||||
if (op.action === "remove") {
|
||||
await validateRemoveTarget(op.target, op.skillsRoot, { repoRoot: REPO_ROOT });
|
||||
await rm(op.target, { recursive: true, force: true });
|
||||
} else if (op.mode === "copy") {
|
||||
if (op.mode === "copy") {
|
||||
await copyDirectoryReplacing(op.source, op.target);
|
||||
} else {
|
||||
await rm(op.target, { recursive: true, force: true });
|
||||
|
||||
Reference in New Issue
Block a user