diff --git a/bin/metamask-skills.mjs b/bin/metamask-skills.mjs index af1ec09a..2dd84b49 100755 --- a/bin/metamask-skills.mjs +++ b/bin/metamask-skills.mjs @@ -11,6 +11,7 @@ const PUBLIC_REPO = 'https://github.com/MetaMask/skills.git'; const CACHE_RELATIVE_DIR = path.join('.skills-cache', 'metamask-skills'); const SOURCE_ENV_KEYS = ['METAMASK_SKILLS_DIR', 'CONSENSYS_SKILLS_DIR']; const TARGET_REPO_ENV_KEY = 'METAMASK_SKILLS_TARGET_REPO'; +const CONTENT_REF_ENV_KEY = 'SKILLS_REF'; function usage(exitCode = 0) { const out = exitCode === 0 ? process.stdout : process.stderr; @@ -304,16 +305,52 @@ function warn(message) { process.stderr.write(`metamask-skills: ${message}\n`); } +/** + * The git ref skill CONTENT is installed from. + * + * A lockfile pins the CLI; on its own it says nothing about which skill revision reaches + * disk. While the cache tracked `main`, a pinned install could still pick up anything + * merged since. Content now follows the release tag matching this package's own version, + * so one lockfile entry pins both halves. + * + * `SKILLS_REF` overrides — for development against `main`, and for holding a consumer on + * a specific release. There is deliberately no automatic widening to `main` when the tag + * is missing: silently falling back to a mutable branch is the behaviour this replaces. + */ +function contentRef(env = process.env) { + const override = env[CONTENT_REF_ENV_KEY]; + if (override) { + return { ref: override, pinned: false, why: `${CONTENT_REF_ENV_KEY}=${override}` }; + } + try { + const { version } = JSON.parse(readFileSync(path.join(PACKAGE_ROOT, 'package.json'), 'utf8')); + if (version) { + return { ref: `v${version}`, pinned: true, why: `package version ${version}` }; + } + } catch { + // fall through + } + return null; +} + function ensurePublicSkillsCache(target) { const cache = cacheDir(target); + const selected = contentRef(); + if (!selected) { + warn('could not determine the pinned content ref; set SKILLS_REF to install skills'); + return false; + } + const { ref, pinned, why } = selected; try { if (isGitDir(cache)) { - const fetchResult = run('git', ['-C', cache, 'fetch', '--depth', '1', 'origin', 'main']); + const fetchResult = run('git', ['-C', cache, 'fetch', '--depth', '1', 'origin', ref]); if (fetchResult.status !== 0) { - warn('cache fetch failed (offline?)'); + // Fail closed. A missing tag means this package version has no published content; + // widening to a branch here would reintroduce the unpinned channel. + warn(`cache fetch failed for ${ref} (${why}) — offline, or the ref does not exist`); return false; } - const resetResult = run('git', ['-C', cache, 'reset', '--hard', 'origin/main']); + const resetResult = run('git', ['-C', cache, 'reset', '--hard', 'FETCH_HEAD']); if (resetResult.status !== 0) { warn('cache reset failed'); return false; @@ -322,11 +359,22 @@ function ensurePublicSkillsCache(target) { } mkdirSync(path.dirname(cache), { recursive: true }); - const cloneResult = run('git', ['clone', '--depth', '1', '--branch', 'main', PUBLIC_REPO, cache]); + const cloneResult = run('git', [ + 'clone', + '--depth', + '1', + '--branch', + ref, + PUBLIC_REPO, + cache, + ]); if (cloneResult.status !== 0) { - warn('cache clone failed (offline?)'); + warn(`cache clone failed for ${ref} (${why}) — offline, or the ref does not exist`); return false; } + if (!pinned) { + warn(`installing skills from ${why}, which is not a pinned release`); + } return true; } catch (error) { warn(`cache refresh failed: ${error instanceof Error ? error.message : String(error)}`); diff --git a/test/cli.test.mjs b/test/cli.test.mjs index e4829a10..a95f9665 100644 --- a/test/cli.test.mjs +++ b/test/cli.test.mjs @@ -1,6 +1,6 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -240,3 +240,42 @@ describe('managed skill pruning', () => { assert.equal(existsSync(stale), true); }); }); + +describe('content ref is pinned', () => { + // A lockfile pins the CLI; it does not pin the skill revision that reaches disk. These + // assert the two are tied together, and that a missing tag fails rather than widening + // to a branch — silently installing from `main` is the behaviour this replaced. + const BIN_SRC = readFileSync( + path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', 'bin', 'metamask-skills.mjs'), + 'utf8', + ); + const BOOTSTRAP_SRC = readFileSync( + path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', 'tools', 'bootstrap'), + 'utf8', + ); + + test('the CLI cache does not track a branch', () => { + assert.ok( + !/'--branch',\s*'main'/u.test(BIN_SRC) && !/origin',\s*'main'/u.test(BIN_SRC), + 'cache clone/fetch still references main directly', + ); + assert.match(BIN_SRC, /function contentRef/u, 'expected a contentRef() resolver'); + }); + + test('the pinned ref matches this package version', () => { + const { version } = JSON.parse( + readFileSync(path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', 'package.json'), 'utf8'), + ); + assert.match(BIN_SRC, /`v\$\{version\}`/u, 'contentRef should derive the tag from package version'); + assert.ok(version, 'package.json must declare a version for the pin to resolve'); + }); + + test('bootstrap does not default to a branch', () => { + assert.ok( + !/REF="\$\{SKILLS_REF:-main\}"/u.test(BOOTSTRAP_SRC), + 'bootstrap still defaults SKILLS_REF to main', + ); + assert.match(BOOTSTRAP_SRC, /latest_release_tag/u, 'expected release-tag resolution'); + assert.match(BOOTSTRAP_SRC, /Refusing to install from an unpinned branch/u, 'expected fail-closed path'); + }); +}); diff --git a/tools/bootstrap b/tools/bootstrap index a8f41eaf..946b06e6 100755 --- a/tools/bootstrap +++ b/tools/bootstrap @@ -18,7 +18,8 @@ # # Env: # SKILLS_CACHE_DIR Where to clone (default: $HOME/.cache/metamask-skills). -# SKILLS_REF Git ref to checkout (default: main). +# SKILLS_REF Git ref to install content from (default: newest release tag). +# Set to `main` to track the branch deliberately. # # Extra args after --repo are forwarded to tools/install. @@ -44,9 +45,37 @@ if [[ -z "$REPO" ]]; then fi CACHE="${SKILLS_CACHE_DIR:-$HOME/.cache/metamask-skills}" -REF="${SKILLS_REF:-main}" TARGET="$(pwd)" +# Resolve the ref to install content from. +# +# This previously defaulted to `main`, so a cloud agent running the documented +# `curl … | bash` one-liner installed whatever had merged since — an unreviewed +# revision by default. It now defaults to the newest release tag, and refuses to +# proceed if none can be resolved rather than falling back to a branch. +# +# Pass SKILLS_REF explicitly for a reproducible install: SKILLS_REF=v0.2.0. +latest_release_tag() { + git ls-remote --tags --refs https://github.com/MetaMask/skills.git 'v*' 2>/dev/null \ + | awk -F/ '{print $NF}' \ + | sort -V \ + | tail -1 +} + +if [[ -n "${SKILLS_REF:-}" ]]; then + REF="$SKILLS_REF" + echo "Using SKILLS_REF=$REF" +else + REF="$(latest_release_tag)" + if [[ -z "$REF" ]]; then + echo "Error: could not resolve a release tag from MetaMask/skills." >&2 + echo "Refusing to install from an unpinned branch. Set SKILLS_REF explicitly" >&2 + echo "(e.g. SKILLS_REF=v0.2.0), or SKILLS_REF=main to track the branch deliberately." >&2 + exit 1 + fi + echo "Using latest release $REF" +fi + if [[ ! -d "$CACHE/.git" ]]; then echo "Cloning MetaMask/skills into $CACHE" mkdir -p "$(dirname "$CACHE")" @@ -54,8 +83,7 @@ if [[ ! -d "$CACHE/.git" ]]; then else echo "Updating $CACHE" git -C "$CACHE" fetch --depth 1 origin "$REF" 2>&1 | sed 's/^/ /' - git -C "$CACHE" checkout "$REF" 2>&1 | sed 's/^/ /' - git -C "$CACHE" reset --hard "origin/$REF" 2>&1 | sed 's/^/ /' + git -C "$CACHE" reset --hard FETCH_HEAD 2>&1 | sed 's/^/ /' fi exec "$CACHE/tools/install" --repo "$REPO" --target "$TARGET" --source "$CACHE" "${EXTRA[@]}"