-
Notifications
You must be signed in to change notification settings - Fork 101
feat(scaffold): auto-detect and install pre-commit tool dependencies #1055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| --- | ||
| # Known pre-commit hook tool dependencies. | ||
| # | ||
| # Used by resolve-precommit-tools.py to auto-detect which system tools | ||
| # a target repo's .pre-commit-config.yaml requires. The resolver reads | ||
| # the target repo's config, matches hook repos and IDs against this | ||
| # registry, and produces a JSON manifest that install-precommit-tools.sh | ||
| # consumes. | ||
| # | ||
| # Structure: | ||
| # Each entry maps a pre-commit hook repo URL to its hooks and the | ||
| # tools they need. Tools are categorized by install method: | ||
| # binary — downloaded from a release URL with SHA256 verification | ||
| # apt — installed via apt-get on Ubuntu-based runners | ||
| # pip — installed via pip | ||
| # npm — installed via npm | ||
| # | ||
| # Binary entries must include pinned versions and per-arch checksums | ||
| # for supply-chain safety. Use the same version+checksum pattern as | ||
| # images/code/Containerfile and the post-scripts. | ||
| # | ||
| # Adding a new tool: | ||
| # 1. Find the hook repo URL and hook ID in .pre-commit-config.yaml | ||
| # 2. Add an entry below with the tools it needs | ||
| # 3. For binary downloads: pin version, provide checksums for amd64+arm64 | ||
| # 4. Run resolve-precommit-tools.py against a test repo to verify | ||
| # | ||
| # Only add entries for hooks that pre-commit cannot self-serve: | ||
| # language: system → tool must already be on PATH (NEEDS registry entry) | ||
| # language: golang → binary download is faster than Go compilation (optional) | ||
| # language: python → pre-commit handles via pip/venv (DO NOT add) | ||
| # language: node → pre-commit handles via npm (DO NOT add) | ||
| # language: docker_image → pre-commit handles via docker pull (DO NOT add) | ||
| # | ||
| # Customization: | ||
| # Per-org: place .pre-commit-tools.yaml in customized/scripts/ | ||
| # Per-repo: place .pre-commit-tools.yaml in .fullsend/customized/scripts/ | ||
| # The customized file completely replaces these defaults. | ||
|
|
||
| tools: | ||
| # ── lychee (markdown link checker) ──────────────────────────────── | ||
| - hook_id: lint-md-links | ||
| repo: local | ||
| match_entry: "lychee" | ||
| install: | ||
| type: binary | ||
| name: lychee | ||
| version: "0.24.2" | ||
| url_template: "https://github.com/lycheeverse/lychee/releases/download/lychee-v{version}/lychee-{triple}.tar.gz" | ||
| checksums: | ||
| x86_64: "1f4e0ef7f6554a6ed33dd7ac144fb2e1bbed98598e7af973042fc5cd43951c9a" | ||
| aarch64: "91a7bd65685da41b90ccb9bc867a3d649a7818042dae04ff405e55a25bddee4c" | ||
| strip_prefix: "lychee-{triple}" | ||
| binary_name: lychee | ||
|
|
||
| # ── gitleaks (secret scanning) ──────────────────────────────────── | ||
| # Post-scripts install gitleaks independently as a security gate. | ||
| # This entry exists only so the resolver recognizes gitleaks hooks | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] correctness The strip_prefix for lychee is set to "lychee-{triple}", but the original hardcoded install extracted the binary from the tarball root. If the tarball has no subdirectory, the find fallback triggers with a spurious warning on every install. Suggested fix: Verify the lychee tarball structure and remove strip_prefix if the binary is at the root. |
||
| # and does NOT emit a "not in registry" warning. The skip_install | ||
| # flag prevents double-installing alongside the post-script copy. | ||
| - hook_id: gitleaks | ||
| repo: https://github.com/zricethezav/gitleaks | ||
| install: | ||
| type: binary | ||
| name: gitleaks | ||
| skip_install: true | ||
|
|
||
| # ── actionlint (GitHub Actions linter) ──────────────────────────── | ||
| # The upstream hook uses language: golang, so pre-commit CAN compile | ||
| # it from source (~2 min). This entry downloads the pre-built binary | ||
| # (~3 sec) to keep the authoritative pre-commit check fast. | ||
| # actionlint releases use "amd64"/"arm64" instead of the Rust-style | ||
| # triple or gitleaks-style "x64". The goarch_override field lets the | ||
| # installer substitute the correct arch string for this tool only. | ||
| - hook_id: actionlint | ||
| repo: https://github.com/rhysd/actionlint | ||
| install: | ||
| type: binary | ||
| name: actionlint | ||
| version: "1.7.11" | ||
| url_template: "https://github.com/rhysd/actionlint/releases/download/v{version}/actionlint_{version}_linux_{goarch}.tar.gz" | ||
| goarch_override: | ||
| x86_64: "amd64" | ||
| aarch64: "arm64" | ||
| checksums: | ||
| x86_64: "900919a84f2229bac68ca9cd4103ea297abc35e9689ebb842c6e34a3d1b01b0a" | ||
| aarch64: "21bc0dfb57a913fe175298c2a9e906ee630f747cb66d0a934d0d4b69f4ee1235" | ||
| binary_name: actionlint | ||
|
|
||
| # ── uv / uvx (Python package manager, needed for ty check) ─────── | ||
| - hook_id: ty | ||
| repo: local | ||
| match_entry: "uvx" | ||
| install: | ||
| type: binary | ||
| name: uv | ||
| version: "0.11.14" | ||
| url_template: "https://github.com/astral-sh/uv/releases/download/{version}/uv-{triple}.tar.gz" | ||
| checksums: | ||
| x86_64: "f3b623eb0e6141a7053d571d59a0bdc341e0f238ea8f5f0b4815ddbec9a2a296" | ||
| aarch64: "c4958f729e216f1610632574ed927b8cf0af1bd02cb88cb30d948571727aee43" | ||
| strip_prefix: "uv-{triple}" | ||
| binary_name: uv | ||
| extra_binaries: | ||
| - uvx | ||
|
|
||
| # Language fallbacks — when a hook is not in the registry above, | ||
| # the resolver uses the hook's `language` field to emit warnings: | ||
| # language: system → warns that the tool must be pre-installed | ||
| # language: golang → warns that Go toolchain is needed | ||
| # language: rust → warns that Rust toolchain is needed | ||
| # Hooks using python/node/docker_image/script need no registry | ||
| # entry — pre-commit handles them natively. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,240 @@ | ||
| #!/usr/bin/env bash | ||
| # Install pre-commit hook dependencies on the GitHub Actions runner. | ||
| # | ||
| # Reads a JSON manifest produced by resolve-precommit-tools.py and | ||
| # installs the listed tools. Supports four install types: | ||
| # binary — download from release URL with SHA256 verification | ||
| # apt — install via apt-get | ||
| # pip — install via pip | ||
| # npm — install via npm -g | ||
| # | ||
| # Binary downloads use architecture detection (uname -m) and pinned | ||
| # checksums for supply-chain safety. Same pattern as post-code.sh and | ||
| # images/code/Containerfile. | ||
| # | ||
| # Usage: | ||
| # install-precommit-tools.sh <manifest.json> | ||
| # | ||
| # The manifest is the JSON output of resolve-precommit-tools.py. | ||
| # | ||
| # Exit codes: | ||
| # 0 — all tools installed (or already present) | ||
| # 1 — critical failure (missing required tool, checksum mismatch) | ||
| set -euo pipefail | ||
|
|
||
| MANIFEST="${1:?Usage: install-precommit-tools.sh <manifest.json>}" | ||
|
|
||
| if [ ! -f "${MANIFEST}" ]; then | ||
| echo "::error::Manifest not found: ${MANIFEST}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| INSTALL_DIR="${HOME}/.local/bin" | ||
| mkdir -p "${INSTALL_DIR}" | ||
| export PATH="${INSTALL_DIR}:${PATH}" | ||
|
|
||
| # Detect architecture once. | ||
| ARCH="$(uname -m)" | ||
| case "${ARCH}" in | ||
| x86_64) | ||
| TRIPLE="x86_64-unknown-linux-gnu" | ||
| GOARCH="x64" | ||
| ;; | ||
| aarch64) | ||
| TRIPLE="aarch64-unknown-linux-gnu" | ||
| GOARCH="arm64" | ||
| ;; | ||
| *) | ||
| echo "::warning::Unsupported architecture: ${ARCH} — skipping binary installs" | ||
| TRIPLE="" | ||
| GOARCH="" | ||
| ;; | ||
| esac | ||
|
|
||
| # Print warnings from the resolver (sanitize to prevent GHA command injection). | ||
| WARNINGS="$(jq -r '.warnings[]' "${MANIFEST}" 2>/dev/null || true)" | ||
| if [ -n "${WARNINGS}" ]; then | ||
| while IFS= read -r w; do | ||
| w="${w//::/ }" | ||
| w="${w//%0A/ }" | ||
| w="${w//%0a/ }" | ||
| w="${w//%0D/ }" | ||
| w="${w//%0d/ }" | ||
| echo "::warning::${w}" | ||
| done <<< "${WARNINGS}" | ||
| fi | ||
|
|
||
| TOOL_COUNT="$(jq '.tools | length' "${MANIFEST}" 2>/dev/null || echo 0)" | ||
| if [ "${TOOL_COUNT}" -eq 0 ]; then | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [info] injection-vuln Warning sanitization handles :: sequences and newline-splitting but does not strip ANSI escape sequences. Attack surface is limited since data originates from the target repo's own .pre-commit-config.yaml. |
||
| echo "No additional pre-commit tools to install" | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Installing ${TOOL_COUNT} pre-commit tool dependency(ies)..." | ||
|
|
||
| # Process each tool entry. | ||
| while IFS= read -r entry; do | ||
| TYPE="$(echo "${entry}" | jq -r '.type')" | ||
| NAME="$(echo "${entry}" | jq -r '.name')" | ||
|
|
||
| # Skip entries marked as handled elsewhere (e.g., gitleaks in post-scripts). | ||
| SKIP="$(echo "${entry}" | jq -r '.skip_install // "false"')" | ||
| if [ "${SKIP}" = "true" ]; then | ||
| echo " ${NAME}: skipped (managed by post-script)" | ||
| continue | ||
| fi | ||
|
|
||
| case "${TYPE}" in | ||
| binary) | ||
| VERSION="$(echo "${entry}" | jq -r '.version')" | ||
| if command -v "${NAME}" >/dev/null 2>&1; then | ||
| INSTALLED_VERSION="$("${NAME}" --version 2>&1 | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -1 || true)" | ||
| if [ "${INSTALLED_VERSION}" = "${VERSION}" ]; then | ||
| echo " ${NAME}: already available v${VERSION} ($(command -v "${NAME}"))" | ||
| continue | ||
| fi | ||
| echo " ${NAME}: found v${INSTALLED_VERSION:-unknown}, need v${VERSION} — installing pinned version" | ||
| fi | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case Version check could match a library version triple in the output instead of the tool's own version, causing a spurious already-installed skip. |
||
|
|
||
| if [ -z "${TRIPLE}" ]; then | ||
| echo "::warning::Cannot install ${NAME} — unsupported architecture" | ||
| continue | ||
| fi | ||
|
|
||
| URL_TEMPLATE="$(echo "${entry}" | jq -r '.url_template')" | ||
| BINARY_NAME="$(echo "${entry}" | jq -r '.binary_name // .name')" | ||
| STRIP_PREFIX="$(echo "${entry}" | jq -r '.strip_prefix // ""')" | ||
|
|
||
| # Resolve checksum for current architecture. | ||
| CHECKSUM="$(echo "${entry}" | jq -r --arg arch "${ARCH}" '.checksums[$arch] // empty')" | ||
| if [ -z "${CHECKSUM}" ]; then | ||
| echo "::warning::No checksum for ${NAME} on ${ARCH} — skipping" | ||
| continue | ||
| fi | ||
|
|
||
| # Resolve per-tool goarch override (e.g., actionlint uses "amd64" not "x64"). | ||
| TOOL_GOARCH="$(echo "${entry}" | jq -r --arg arch "${ARCH}" '.goarch_override[$arch] // empty')" | ||
| if [ -z "${TOOL_GOARCH}" ]; then | ||
| TOOL_GOARCH="${GOARCH}" | ||
| fi | ||
|
|
||
| # Resolve URL template. | ||
| URL="${URL_TEMPLATE}" | ||
| URL="${URL//\{version\}/${VERSION}}" | ||
| URL="${URL//\{triple\}/${TRIPLE}}" | ||
| URL="${URL//\{goarch\}/${TOOL_GOARCH}}" | ||
|
|
||
| echo " ${NAME} v${VERSION}: downloading..." | ||
| DL_TMPDIR="$(mktemp -d)" | ||
| TARBALL="${DL_TMPDIR}/${NAME}.tar.gz" | ||
|
|
||
| if ! curl -fsSL "${URL}" -o "${TARBALL}"; then | ||
| echo "::warning::Failed to download ${NAME} v${VERSION} — skipping" | ||
| rm -rf "${DL_TMPDIR}" | ||
| continue | ||
| fi | ||
| if ! echo "${CHECKSUM} ${TARBALL}" | sha256sum -c -; then | ||
| echo "::error::Checksum verification failed for ${NAME} v${VERSION}" | ||
| rm -rf "${DL_TMPDIR}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| if ! tar xzf "${TARBALL}" -C "${DL_TMPDIR}"; then | ||
| echo "::warning::Failed to extract ${NAME} archive — skipping" | ||
| rm -rf "${DL_TMPDIR}" | ||
| continue | ||
| fi | ||
|
|
||
| # Find and install the binary. | ||
| if [ -n "${STRIP_PREFIX}" ]; then | ||
| RESOLVED_PREFIX="${STRIP_PREFIX//\{triple\}/${TRIPLE}}" | ||
| RESOLVED_PREFIX="${RESOLVED_PREFIX//\{version\}/${VERSION}}" | ||
| BIN_PATH="${DL_TMPDIR}/${RESOLVED_PREFIX}/${BINARY_NAME}" | ||
| else | ||
| BIN_PATH="${DL_TMPDIR}/${BINARY_NAME}" | ||
| fi | ||
|
|
||
| if [ ! -f "${BIN_PATH}" ]; then | ||
| echo "::warning::Binary not found at expected path: ${BIN_PATH}" | ||
| FOUND="$(find "${DL_TMPDIR}" -name "${BINARY_NAME}" -type f | head -1)" | ||
| if [ -n "${FOUND}" ]; then | ||
| BIN_PATH="${FOUND}" | ||
| else | ||
| echo "::error::Cannot find ${BINARY_NAME} in archive" | ||
| rm -rf "${DL_TMPDIR}" | ||
| continue | ||
| fi | ||
| fi | ||
|
|
||
| if ! mv "${BIN_PATH}" "${INSTALL_DIR}/${BINARY_NAME}"; then | ||
| echo "::warning::Failed to install ${NAME} binary — skipping" | ||
| rm -rf "${DL_TMPDIR}" | ||
| continue | ||
| fi | ||
| chmod +x "${INSTALL_DIR}/${BINARY_NAME}" | ||
|
|
||
| # Install extra binaries (e.g., uvx alongside uv). | ||
| EXTRAS="$(echo "${entry}" | jq -r '.extra_binaries[]? // empty' 2>/dev/null || true)" | ||
| if [ -n "${EXTRAS}" ]; then | ||
| while IFS= read -r extra; do | ||
| EXTRA_PATH="" | ||
| if [ -n "${STRIP_PREFIX}" ]; then | ||
| EXTRA_PATH="${DL_TMPDIR}/${RESOLVED_PREFIX}/${extra}" | ||
| fi | ||
| if [ ! -f "${EXTRA_PATH:-}" ]; then | ||
| EXTRA_PATH="$(find "${DL_TMPDIR}" -name "${extra}" -type f | head -1)" | ||
| fi | ||
| if [ -n "${EXTRA_PATH}" ] && [ -f "${EXTRA_PATH}" ]; then | ||
| mv "${EXTRA_PATH}" "${INSTALL_DIR}/${extra}" | ||
| chmod +x "${INSTALL_DIR}/${extra}" | ||
| echo " ${NAME}: installed extra binary: ${extra}" | ||
| fi | ||
| done <<< "${EXTRAS}" | ||
| fi | ||
|
|
||
| rm -rf "${DL_TMPDIR}" | ||
| echo " ${NAME} v${VERSION}: installed to ${INSTALL_DIR}/${BINARY_NAME}" | ||
| ;; | ||
|
|
||
| apt) | ||
| if command -v "${NAME}" >/dev/null 2>&1; then | ||
| echo " ${NAME}: already available" | ||
| continue | ||
| fi | ||
| echo " ${NAME}: installing via apt-get..." | ||
| sudo apt-get update -qq && sudo apt-get install -y -qq "${NAME}" 2>/dev/null \ | ||
| || echo "::warning::Failed to install ${NAME} via apt-get" | ||
| ;; | ||
|
|
||
| pip) | ||
| VERSION="$(echo "${entry}" | jq -r '.version // ""')" | ||
| if [ -z "${VERSION}" ]; then | ||
| echo "::warning::No version pinned for pip package ${NAME} — skipping for supply-chain safety" | ||
| continue | ||
| fi | ||
| PKG="${NAME}==${VERSION}" | ||
| echo " ${NAME}: installing via pip..." | ||
| pip install --quiet --no-deps --break-system-packages "${PKG}" 2>/dev/null \ | ||
| || pip3 install --quiet --no-deps --break-system-packages "${PKG}" 2>/dev/null \ | ||
| || echo "::warning::Failed to install ${NAME} via pip" | ||
| ;; | ||
|
|
||
| npm) | ||
| VERSION="$(echo "${entry}" | jq -r '.version // ""')" | ||
| if [ -z "${VERSION}" ]; then | ||
| echo "::warning::No version pinned for npm package ${NAME} — skipping for supply-chain safety" | ||
| continue | ||
| fi | ||
| NPM_PKG="${NAME}@${VERSION}" | ||
| echo " ${NAME}: installing via npm..." | ||
| npm install -g --ignore-scripts "${NPM_PKG}" 2>/dev/null \ | ||
| || echo "::warning::Failed to install ${NAME} via npm" | ||
| ;; | ||
|
|
||
| *) | ||
| echo "::warning::Unknown install type '${TYPE}' for ${NAME}" | ||
| ;; | ||
| esac | ||
| done < <(jq -c '.tools[]' "${MANIFEST}") | ||
|
|
||
| echo "Pre-commit tool installation complete" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be top level? Now it will be distributed/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question — moved it from
tools/toscripts/in a follow-up push but forgot to reply here.The registry lives under
scripts/becauseresolve-precommit-tools.pylooks it up relative to its own directory (os.path.join(script_dir, "precommit-tools.yaml")), so co-locating them keeps the lookup simple with no extra path configuration.Also,
scripts/is a layered directory inscaffold.go— it gets distributed at runtime via reusable workflows, not installed into.fullsendrepos. Putting it at the top level would mean it gets scaffolded into every org's config repo, which isn't the intent.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But having it as a layered directory does the same as being installed into repositories from the practical point of view, no? The script will install OUR tools for all repositories using fullsend. We need to move this to toplevel, so the script will get layered, but the definition of OUR tools won't. Users of fullsend can take advantage of this creating its own
precommit-tools.yamlat the root level.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean — since
scripts/is layered, the registry does travel with the scripts at runtime. But I think that's the right behavior for this PR's scope, and here's why:What this PR replaces: The hardcoded
LYCHEE_VERSION/LYCHEE_SHA256_AMD64/UV_VERSION/UV_SHA256blocks that were previously inline inpost-code.shandpost-fix.sh. Those were also distributed via the same layeredscripts/path. The registry is the same data externalized into a structured file — it doesn't change what gets distributed, just how it's organized.What the registry is: Fullsend's knowledge of which pre-commit hook repos need which system tools. It's infrastructure knowledge, not user configuration. Users don't need to know (or care) that
lycheeneeds a specific binary download with a specific checksum — that's fullsend's job.User-provided overrides are #1270 territory. The resolver already takes
registry_pathas a parameter (not hardcoded inresolve()), so adding a merge step later — load fullsend's built-in registry, overlay a user-providedprecommit-tools.yamlfrom the target repo — is straightforward. But that's a different feature with different requirements (merge semantics, conflict resolution, validation).This PR needs to land so the follow-up work can proceed — #1270 (registry expansion), #836 (shared tool install logic), #850 (pre-flight checks), and #1056/#1057 (review findings) are all blocked on it. I'd rather not widen the scope further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about shellcheck and actionlint? Those are in the file, but they weren't on post-code. They are currently compiled by golang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call — you're right that shellcheck and actionlint weren't in the old hardcoded blocks.
shellcheck: removed. Both
shellcheck-py/shellcheck-py(language: python) andkoalaman/shellcheck-precommit(language: docker_image) are self-managed by pre-commit — it installs shellcheck-py via pip/venv or pulls the Docker image. The apt-installed system shellcheck was wasted work and could cause version skew with the pip-bundled binary. Dropped both entries.actionlint: kept with rationale comment. The hook uses
language: golang, so pre-commit CAN compile it from source, but that takes ~2 minutes on GHA runners. The registry downloads the pre-built binary in ~3 seconds. It's a performance optimization, not a correctness fix. Added an explicit comment making this trade-off visible. If you'd rather let pre-commit handle it natively and accept the build time, I can drop it too.Also added a comment to the registry header documenting when entries are appropriate (only for hooks pre-commit can't self-serve) and how to customize (place
.pre-commit-tools.yamlincustomized/scripts/for full replacement). Additive merge (adding/suppressing individual entries without replacing the whole file) is follow-up territory (#1270).