From d6b3e823dac1efe7807c33c85aaa5afdd572c678 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 25 Oct 2026 13:59:58 -0400 Subject: [PATCH 1/6] fix(conformance): permanent per-port class for deliberate public ports (D-0626) public-ports.yaml names the plugin and the exact container port; port-bind passes only those ports, so the traefik exception no longer excuses every port of the plugin and no longer expires at the v1.5.0 cap. --- .github/wiki/Plugin-Conformance.md | 4 +- scripts/conformance/check.sh | 8 ++-- scripts/conformance/exceptions.yaml | 6 --- scripts/conformance/fixtures.sh | 18 +++++++ scripts/conformance/portbind/main.go | 58 +++++++++++++++++++---- scripts/conformance/portbind/main_test.go | 54 +++++++++++++++++++++ scripts/conformance/public-ports.yaml | 15 ++++++ scripts/conformance/rules/exceptions.sh | 15 +++++- scripts/conformance/rules/port-bind.sh | 12 +++-- 9 files changed, 166 insertions(+), 24 deletions(-) create mode 100644 scripts/conformance/public-ports.yaml diff --git a/.github/wiki/Plugin-Conformance.md b/.github/wiki/Plugin-Conformance.md index f6bf99a5b..ee3f1a410 100644 --- a/.github/wiki/Plugin-Conformance.md +++ b/.github/wiki/Plugin-Conformance.md @@ -27,7 +27,7 @@ Exit 0 means every selected rule passed. A rule that cannot run (no Docker, no t | `healthcheck` | A compose plugin has a real health check: a compose `healthcheck:` that is not disabled (`disable: true`, `test: NONE`), or a Dockerfile `HEALTHCHECK` whose last instruction is not `NONE`. A disabled compose healthcheck fails even when the Dockerfile has one. | | `replace` | No `go.mod` or `go.work` `replace` or `use` points outside the plugin directory, quoted targets included. | | `docker-socket` | No `docker.sock`, `/var/run/docker` or `/var/run` bind in the compose fragment named by `service.compose` (whatever it is called), in any `*.yml` or `*.yaml` of the plugin, or in a Dockerfile (ADR 0027), unless a dated exception names the plugin. | -| `port-bind` | Every published port of every compose service binds `127.0.0.1`: in the `service.compose` fragment (which must parse) and in any other `*.yml` or `*.yaml` of the plugin (outside `vendor/`, `node_modules/`, `third_party/` and `testdata/`) that has a top-level `services:` map. The parser is `scripts/conformance/portbind` (Go). A port with no host IP (`"9007:9007"`, `"9007"`), `0.0.0.0`, an IPv6 address, a host IP taken from a variable, a long-syntax entry without `host_ip: 127.0.0.1` and `network_mode: host` all fail. A deliberate public port needs a dated `port-bind` exception (today: `traefik`, 80 and 443). | +| `port-bind` | Every published port of every compose service binds `127.0.0.1`: in the `service.compose` fragment (which must parse) and in any other `*.yml` or `*.yaml` of the plugin (outside `vendor/`, `node_modules/`, `third_party/` and `testdata/`) that has a top-level `services:` map. The parser is `scripts/conformance/portbind` (Go). A port with no host IP (`"9007:9007"`, `"9007"`), `0.0.0.0`, an IPv6 address, a host IP taken from a variable, a long-syntax entry without `host_ip: 127.0.0.1` and `network_mode: host` all fail. A deliberate public port is listed in `scripts/conformance/public-ports.yaml` (plugin, one container port, reason; no expiry; checked per port, so every other port of that plugin still has to bind `127.0.0.1`; `network_mode: host` is never excused). Today: `traefik` 80, 443 and 8444. The `exceptions` rule validates the file. A dated `port-bind` exception in `exceptions.yaml` still excuses a whole plugin. | | `migrations` | A plugin with `migrations/*.sql` declares `migrations.apply: boot`, and a Go plugin calls `migrate.Apply` at boot in non-test source: a call in a comment or only in a `_test.go` file does not count. | | `boot-schema` | A manifest with `migrations.apply: boot` declares `schema: np_`, the schema that holds its boot ledger. A `Schema: "np_..."` literal in non-test Go source must name the same schema. | | `migration-immutable` | A boot migration file that the last release shipped is frozen: editing, deleting or renaming it fails, because the boot ledger stores each applied file's sha256 and a changed file is fatal at boot. Only plugins that the baseline release already applied at boot (manifest v2 with `migrations.apply: boot` at that tag) are checked; earlier releases had no ledger. Fix: add a new file. | @@ -41,7 +41,7 @@ Exit 0 means every selected rule passed. A rule that cannot run (no Docker, no t ## Exceptions -`scripts/conformance/exceptions.yaml` lists the only allowed rule breaks. Each entry has `rule`, `plugin`, `reason`, `owner_ref` and `expires`. An expired entry fails the gate, and `expires` may not be later than the v1.5.0 cap date. The cap is a constant in `scripts/conformance/lib.sh` (`release_cap`), not read from the data file, so a file cannot raise its own cap. Remove an entry in the same change that lands its owner Ticket. Today: `docker-socket` for `forgejo` and `monitoring`, `port-bind` for `traefik` (the edge proxy publishes 80 and 443 by design), and `cli-targets` entries for plugins whose release builds no binary yet (see the file). +`scripts/conformance/exceptions.yaml` lists the only allowed rule breaks. Each entry has `rule`, `plugin`, `reason`, `owner_ref` and `expires`. An expired entry fails the gate, and `expires` may not be later than the v1.5.0 cap date. The cap is a constant in `scripts/conformance/lib.sh` (`release_cap`), not read from the data file, so a file cannot raise its own cap. Remove an entry in the same change that lands its owner Ticket. Today: `docker-socket` for `forgejo` and `monitoring`, and `cli-targets` entries for plugins whose release builds no binary yet (see the file). ## Pinned schema diff --git a/scripts/conformance/check.sh b/scripts/conformance/check.sh index a75867fef..d6ec9af42 100644 --- a/scripts/conformance/check.sh +++ b/scripts/conformance/check.sh @@ -17,13 +17,13 @@ # not a pass; --changed with nothing changed passes). set -u . "$(dirname "$0")/lib.sh" -mode=all; list=; root=$CONF_ROOT/free; rules=; exc=$CONF_DIR/exceptions.yaml +mode=all; list=; root=$CONF_ROOT/free; rules=; exc=$CONF_DIR/exceptions.yaml; pub=$CONF_DIR/public-ports.yaml ALL="schema compat healthcheck replace docker-socket port-bind migrations boot-schema migration-immutable migration-idempotent legacy-renames repository ports build" migbase=; shard= while [ $# -gt 0 ]; do case $1 in --plugins) mode=plugins; list=$2; shift;; --changed) mode=changed;; --all) mode=all;; --fixtures) mode=fixtures;; --rules) rules=$(printf '%s' "$2" | tr ',' ' '); shift;; - --root) root=$2; shift;; --exceptions) exc=$2; shift;; --migration-base) migbase=$2; shift;; --shard) shard=$2; shift;; + --root) root=$2; shift;; --exceptions) exc=$2; shift;; --public-ports) pub=$2; shift;; --migration-base) migbase=$2; shift;; --shard) shard=$2; shift;; *) echo "check.sh: unknown argument $1" >&2; exit 2;; esac; shift; done [ -n "$rules" ] || rules="exceptions $ALL" if [ "$mode" = fixtures ]; then exec bash "$CONF_DIR/fixtures.sh"; fi @@ -65,7 +65,7 @@ case " $rules " in *" migration-immutable "*|*" migration-idempotent "*) fi [ -z "$migbase" ] || echo "migration baseline: $CONF_MIG_LABEL";; esac -case " $rules " in *" exceptions "*) bash "$CONF_DIR/rules/exceptions.sh" "$exc" || rc=1;; esac +case " $rules " in *" exceptions "*) bash "$CONF_DIR/rules/exceptions.sh" "$exc" "$pub" || rc=1;; esac for p in $(printf '%s' "$list" | tr ',' ' '); do dir=$root/$p if [ ! -f "$dir/plugin.json" ]; then @@ -79,7 +79,7 @@ for p in $(printf '%s' "$list" | tr ',' ' '); do case $r in exceptions) continue;; build) cmd="bash $CONF_DIR/build-from-tarball.sh $dir";; ports) cmd="bash $CONF_DIR/ports.sh --plugins $p --root $root";; docker-socket) cmd="bash $CONF_DIR/rules/docker-socket.sh $dir $exc";; - port-bind) cmd="bash $CONF_DIR/rules/port-bind.sh $dir $exc";; + port-bind) cmd="bash $CONF_DIR/rules/port-bind.sh $dir $exc $pub";; migration-immutable|migration-idempotent) [ -n "$migbase" ] || continue; cmd="bash $CONF_DIR/rules/$r.sh $dir $exc $migbase";; legacy-renames) cmd="bash $CONF_DIR/rules/legacy-renames.sh $dir $exc";; *) cmd="bash $CONF_DIR/rules/$r.sh $dir";; esac diff --git a/scripts/conformance/exceptions.yaml b/scripts/conformance/exceptions.yaml index f960531ba..842f760c6 100644 --- a/scripts/conformance/exceptions.yaml +++ b/scripts/conformance/exceptions.yaml @@ -58,12 +58,6 @@ exceptions: reason: "core still registers ci eval until the break-out ships (temporary duplicate, P7-CI-44)" owner_ref: P7-SHIP-09 expires: 2026-12-31 - # Rule port-bind (D-0453): a deliberate public port. D-0626 tracks a permanent class for these. - - rule: port-bind - plugin: traefik - reason: "the edge proxy publishes 80 and 443 on all interfaces by design (NSELF_HTTP_PORT, NSELF_HTTPS_PORT)" - owner_ref: D-0626 - expires: 2026-12-31 # Rule migration-idempotent (D-0619): the stricter DO-block lint no longer treats a BEGIN..EXCEPTION block whose handler # catches unrelated errors as replay protection. webhooks 002 runs `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` through # EXECUTE format() in a loop (replay-safe, but the lint cannot read dynamic SQL). diff --git a/scripts/conformance/fixtures.sh b/scripts/conformance/fixtures.sh index 0eb959d40..e67083c83 100644 --- a/scripts/conformance/fixtures.sh +++ b/scripts/conformance/fixtures.sh @@ -180,6 +180,24 @@ printf 'exceptions:\n - rule: port-bind\n plugin: beta\n reason: "fixture printf 'exceptions:\n - rule: port-bind\n plugin: beta\n reason: "fixture"\n owner_ref: P7-TEST\n expires: 2020-01-01\n' > "$work/exc-pb-old.yaml" pass_plant port-all-interfaces-excepted beta port-bind "setport ' - \"3902:3902\"'" --exceptions "$work/exc-pb-ok.yaml" plant port-all-interfaces-expired beta port-bind "setport ' - \"3902:3902\"'" --exceptions "$work/exc-pb-old.yaml" +# Rule port-bind, public-ports.yaml (D-0626): a listed container port passes on any host IP, checked per port; another port of the +# same plugin, the same port of another plugin and network_mode host still fail; a malformed entry fails the exceptions rule. +pubfile() { printf 'public_ports:\n - plugin: %s\n port: %s\n reason: "fixture"\n' "$1" "$2" > "$work/pub-$1-$2.yaml"; printf '%s\n' "$work/pub-$1-$2.yaml"; } +pass_plant port-public-listed beta port-bind "setport ' - \"3902:3902\"'" --public-ports "$(pubfile beta 3902)" +pass_plant port-public-listed-any-ip beta port-bind "setport ' - \"0.0.0.0:3902:3902\"'" --public-ports "$(pubfile beta 3902)" +plant port-public-other-port beta port-bind "setport ' - \"3902:3902\"\n - \"3903:3903\"'" --public-ports "$(pubfile beta 3902)" +plant port-public-other-plugin beta port-bind "setport ' - \"3902:3902\"'" --public-ports "$(pubfile alpha 3902)" +plant port-public-host-network beta port-bind "setport ' - \"3902:3902\"'; hostnet" --public-ports "$(pubfile beta 3902)" +plant port-public-file-missing beta port-bind "setport ' - \"3902:3902\"'" --public-ports "$work/pub-nonexistent.yaml" +for bad in 'port: 0' 'port: 70000' 'port: 80-90' 'port: ""' 'port: eighty'; do + printf 'public_ports:\n - plugin: beta\n %s\n reason: "fixture"\n' "$bad" > "$work/pub-bad.yaml" + out=$(bash "$CONF_DIR/check.sh" --root "$conv" --plugins beta --rules exceptions --public-ports "$work/pub-bad.yaml" 2>&1) && { echo "FAIL public-ports entry ($bad) passed"; rc=1; } \ + || { printf '%s\n' "$out" | grep -q 'FAIL public-ports beta' && echo "pass public-ports entry ($bad) fails the gate" || { echo "FAIL public-ports entry ($bad) failed for the wrong reason"; printf '%s\n' "$out" | sed 's/^/ /'; rc=1; }; } +done +printf 'public_ports:\n - plugin: beta\n port: 80\n reason: "fixture"\n - plugin: beta\n port: 80\n reason: "again"\n' > "$work/pub-dup.yaml" +out=$(bash "$CONF_DIR/check.sh" --root "$conv" --plugins beta --rules exceptions --public-ports "$work/pub-dup.yaml" 2>&1) && { echo "FAIL a duplicate public-ports entry passed"; rc=1; } || { printf '%s\n' "$out" | grep -q 'listed twice' && echo "pass a duplicate public-ports entry fails the gate" || { echo "FAIL duplicate public-ports failed for the wrong reason"; rc=1; }; } +# The committed file passes (traefik 80 and 443 only) and outlives the exceptions cap: it has no expiry. +out=$(CONF_TODAY=2027-06-01 bash "$CONF_DIR/check.sh" --root "$conv" --plugins beta --rules exceptions --exceptions "$work/exc-ok.yaml" 2>&1 | grep 'public-ports'); [ -z "$out" ] && echo "pass committed public-ports.yaml does not expire" || { echo "FAIL committed public-ports.yaml failed after the cap date: $out"; rc=1; } out=$(bash "$CONF_DIR/check.sh" --root "$conv" --plugins beta --rules exceptions --exceptions "$work/exc-old.yaml" 2>&1) && { echo "FAIL expired exception did not fail the gate"; rc=1; } || { printf '%s\n' "$out" | grep -q 'FAIL exceptions' && echo "pass expired exception fails the gate" || { echo "FAIL expired exception rc but no message"; rc=1; }; } out=$(CONF_TODAY=2027-06-01 bash "$CONF_DIR/check.sh" --root "$conv" --plugins beta --rules exceptions 2>&1) && { echo "FAIL committed exceptions.yaml must expire"; rc=1; } || echo "pass committed exceptions expire after the cap date" [ $rc -eq 0 ] && echo "fixtures: gate ok" || echo "fixtures: gate FAIL" diff --git a/scripts/conformance/portbind/main.go b/scripts/conformance/portbind/main.go index 2a93f19ec..7468accfb 100644 --- a/scripts/conformance/portbind/main.go +++ b/scripts/conformance/portbind/main.go @@ -1,6 +1,8 @@ // Command portbind checks that every published port of every compose service binds 127.0.0.1. // -// Usage: portbind [-require FILE]... FILE... +// Usage: portbind [-allow PORT]... [-require FILE]... FILE... +// -allow PORT names a container port the plugin may publish on any host IP (a deliberate public port, listed in +// scripts/conformance/public-ports.yaml); the match is on the exact target port, so every other port still has to bind 127.0.0.1. // Prints one line per violation (": service ") and exits 0 when there is none, // 1 when there is at least one, 2 on a usage or I/O error. A -require file must parse and hold a top-level // `services:` mapping, otherwise the tool exits 2 (fail closed). Other files that do not parse as YAML or @@ -16,6 +18,7 @@ import ( "os" "path/filepath" "sort" + "strconv" "strings" "gopkg.in/yaml.v3" @@ -26,6 +29,7 @@ func main() { os.Exit(run(os.Args[1:], os.Stdout, os.Stderr)) } // run is the testable entry point. func run(args []string, stdout, stderr io.Writer) int { var required, files []string + allow := map[string]bool{} for i := 0; i < len(args); i++ { switch a := args[i]; { case a == "-require" || a == "--require": @@ -35,6 +39,17 @@ func run(args []string, stdout, stderr io.Writer) int { } i++ required = append(required, args[i]) + case a == "-allow" || a == "--allow": + if i+1 >= len(args) { + fmt.Fprintln(stderr, "portbind: -allow needs a port") + return 2 + } + i++ + if n, err := strconv.Atoi(args[i]); err != nil || n < 1 || n > 65535 { + fmt.Fprintf(stderr, "portbind: -allow %q is not a port number\n", args[i]) + return 2 + } + allow[args[i]] = true case strings.HasPrefix(a, "-") && a != "-": fmt.Fprintf(stderr, "portbind: unknown flag %s\n", a) return 2 @@ -43,7 +58,7 @@ func run(args []string, stdout, stderr io.Writer) int { } } if len(required) == 0 && len(files) == 0 { - fmt.Fprintln(stderr, "usage: portbind [-require FILE]... FILE...") + fmt.Fprintln(stderr, "usage: portbind [-allow PORT]... [-require FILE]... FILE...") return 2 } // One check per file: a required file wins over the same path given as a plain file. @@ -72,7 +87,7 @@ func run(args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "portbind: %v\n", err) return 2 } - vs, hasServices, perr := checkFile(data, j.req) + vs, hasServices, perr := checkFile(data, j.req, allow) if j.req && (perr != nil || !hasServices) { if perr == nil { perr = errors.New("no top-level services mapping") @@ -96,7 +111,7 @@ func run(args []string, stdout, stderr io.Writer) int { // strict is true for a -require file. A long-syntax ports entry without `target` is not valid compose (the // compose spec requires target); outside strict mode it is skipped, because Helm values files also have a // top-level `services` mapping whose `ports` entries look like {port: 80, protocol: TCP}. -func checkFile(data []byte, strict bool) (vs []string, hasServices bool, err error) { +func checkFile(data []byte, strict bool, allow map[string]bool) (vs []string, hasServices bool, err error) { dec := yaml.NewDecoder(bytes.NewReader(data)) for { var doc any @@ -121,7 +136,7 @@ func checkFile(data []byte, strict bool) (vs []string, hasServices bool, err err } sort.Strings(names) for _, n := range names { - for _, d := range checkService(svcs[n], strict) { + for _, d := range checkService(svcs[n], strict, allow) { vs = append(vs, fmt.Sprintf("service %s %s", n, d)) } } @@ -129,7 +144,7 @@ func checkFile(data []byte, strict bool) (vs []string, hasServices bool, err err } // checkService returns one description per violation of a single service definition. -func checkService(def any, strict bool) []string { +func checkService(def any, strict bool, allow map[string]bool) []string { if def == nil { return nil } @@ -150,7 +165,7 @@ func checkService(def any, strict bool) []string { return append(out, "has a ports value that is not a list (cannot verify)") } for _, p := range list { - if d := checkPort(p, strict); d != "" { + if d := checkPort(p, strict, allow); d != "" { out = append(out, d) } } @@ -158,16 +173,26 @@ func checkService(def any, strict bool) []string { } // checkPort returns the violation description for one ports entry, or "" when it binds 127.0.0.1. -func checkPort(p any, strict bool) string { +// A port whose container port is in allow passes whatever its host IP. +func checkPort(p any, strict bool, allow map[string]bool) string { switch v := p.(type) { case string: + if allow[shortTarget(v)] { + return "" + } return checkShort(v) case int: + if allow[fmt.Sprint(v)] { + return "" + } return checkShort(fmt.Sprint(v)) case map[string]any: if _, ok := v["target"]; !ok && !strict { return "" } + if t, ok := v["target"]; ok && allow[fmt.Sprint(t)] { + return "" + } return checkLong(v) default: return fmt.Sprintf("has a ports entry %v of a type that cannot be verified", p) @@ -196,6 +221,23 @@ func checkLong(m map[string]any) string { return desc + " on " + s } +// shortTarget returns the container port of a short-syntax entry ("" when it is a range or cannot be read): +// the last colon-separated part with any /tcp or /udp suffix removed. +func shortTarget(raw string) string { + spec := strings.TrimSpace(raw) + for _, suf := range []string{"/tcp", "/udp"} { + spec = strings.TrimSuffix(spec, suf) + } + parts := splitPorts(spec) + if len(parts) < 1 || len(parts) > 3 { + return "" + } + if _, err := strconv.Atoi(parts[len(parts)-1]); err != nil { + return "" + } + return parts[len(parts)-1] +} + func checkShort(raw string) string { spec := strings.TrimSpace(raw) for _, suf := range []string{"/tcp", "/udp"} { diff --git a/scripts/conformance/portbind/main_test.go b/scripts/conformance/portbind/main_test.go index 68a11ba67..a7855c305 100644 --- a/scripts/conformance/portbind/main_test.go +++ b/scripts/conformance/portbind/main_test.go @@ -147,3 +147,57 @@ func TestSplitPorts(t *testing.T) { } } } + +// -allow (D-0626): a listed container port passes on any host IP; every other port of the same service still fails. +func TestAllow(t *testing.T) { + cases := []struct { + name string + ports string + args []string + rc int + want string // substring that must appear in stdout + }{ + {"listed ports pass", `["${HTTP:-80}:80", "${HTTPS:-443}:443"]`, []string{"-allow", "80", "-allow", "443"}, 0, ""}, + {"unlisted port beside a listed one fails", `["80:80", "8444:8444"]`, []string{"-allow", "80"}, 1, `"8444:8444"`}, + {"without -allow the same entry fails", `["80:80"]`, nil, 1, "on all interfaces"}, + {"match is on the container port, not the host port", `["80:8080"]`, []string{"-allow", "80"}, 1, `"80:8080"`}, + {"container-only form", `["443"]`, []string{"-allow", "443"}, 0, ""}, + {"tcp suffix", `["80:80/tcp"]`, []string{"-allow", "80"}, 0, ""}, + {"explicit host ip", `["0.0.0.0:80:80"]`, []string{"-allow", "80"}, 0, ""}, + {"range is not a listed port", `["80-90:80-90"]`, []string{"-allow", "80"}, 1, "80-90"}, + {"variable container port is not a listed port", `["80:${P:-80}"]`, []string{"-allow", "80"}, 1, "80:${P:-80}"}, + {"long syntax", "\n - target: 80\n published: 80\n", []string{"-allow", "80"}, 0, ""}, + {"long syntax unlisted target", "\n - target: 81\n published: 80\n", []string{"-allow", "80"}, 1, "target 81"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + f := filepath.Join(t.TempDir(), "c.yml") + if err := os.WriteFile(f, []byte(head+" ports: "+c.ports+"\n"), 0o644); err != nil { + t.Fatal(err) + } + var out, errb bytes.Buffer + if rc := run(append(append([]string{}, c.args...), f), &out, &errb); rc != c.rc { + t.Fatalf("rc = %d, want %d\nstdout: %s\nstderr: %s", rc, c.rc, out.String(), errb.String()) + } + if c.want != "" && !strings.Contains(out.String(), c.want) { + t.Errorf("stdout lacks %q:\n%s", c.want, out.String()) + } + }) + } + t.Run("allow never excuses network_mode host", func(t *testing.T) { + f := filepath.Join(t.TempDir(), "c.yml") + if err := os.WriteFile(f, []byte(head+" network_mode: host\n ports: [\"80:80\"]\n"), 0o644); err != nil { + t.Fatal(err) + } + var out, errb bytes.Buffer + if rc := run([]string{"-allow", "80", f}, &out, &errb); rc != 1 || !strings.Contains(out.String(), "network_mode host") { + t.Fatalf("rc = %d\n%s", rc, out.String()) + } + }) + for _, bad := range [][]string{{"-allow"}, {"-allow", "x"}, {"-allow", "0"}, {"-allow", "70000"}, {"-allow", "80-90"}} { + var out, errb bytes.Buffer + if rc := run(append(bad, "f.yml"), &out, &errb); rc != 2 { + t.Errorf("run(%v) = %d, want 2", bad, rc) + } + } +} diff --git a/scripts/conformance/public-ports.yaml b/scripts/conformance/public-ports.yaml new file mode 100644 index 000000000..306379d01 --- /dev/null +++ b/scripts/conformance/public-ports.yaml @@ -0,0 +1,15 @@ +# Deliberate public ports for the port-bind rule (D-0626). Every other published port of a plugin must bind 127.0.0.1. +# An entry names the plugin and ONE container port (the right-hand side of "host:container", or `target`); that port may +# be published on any host IP. Unlike exceptions.yaml an entry does not expire: it is the permanent class for a plugin +# whose job is to be reachable. Review it like reserved-ports.yaml: a new entry needs a reason a reviewer can check. +# Fields: plugin, port (1-65535, a single number), reason. network_mode host is never allowed. +public_ports: + - plugin: traefik + port: 80 + reason: "the edge proxy serves HTTP on all interfaces by design (NSELF_HTTP_PORT)" + - plugin: traefik + port: 443 + reason: "the edge proxy serves HTTPS on all interfaces by design (NSELF_HTTPS_PORT)" + - plugin: traefik + port: 8444 + reason: "the upload entrypoint binds loopback unless the operator sets TRAEFIK_UPLOAD_PUBLISH to open it to a public upload route" diff --git a/scripts/conformance/rules/exceptions.sh b/scripts/conformance/rules/exceptions.sh index 9397725b2..5df40fa22 100644 --- a/scripts/conformance/rules/exceptions.sh +++ b/scripts/conformance/rules/exceptions.sh @@ -1,8 +1,10 @@ #!/usr/bin/env bash # Rule exceptions (D13): every entry is complete, expires on or before the cap and has not expired. +# Also checks public-ports.yaml (D-0626, the permanent class for deliberate public ports): every entry has a plugin, a reason +# and one port number from 1 to 65535, and no entry repeats. These entries do not expire. set -u . "$(dirname "$0")/../lib.sh" -file=${1:-$CONF_DIR/exceptions.yaml}; today=$(conf_today); rc=0 +file=${1:-$CONF_DIR/exceptions.yaml}; pub=${2:-$CONF_DIR/public-ports.yaml}; today=$(conf_today); rc=0 cap=$(release_cap) # fixed in lib.sh; a "# cap:" comment in the data file is ignored while IFS="$(printf '\t')" read -r rule plugin reason owner exp; do [ -n "$rule$plugin$reason$owner$exp" ] || continue @@ -12,4 +14,15 @@ while IFS="$(printf '\t')" read -r rule plugin reason owner exp; do done </dev/null) kind=$(jq -r '.service.kind // empty' "$dir/plugin.json" 2>/dev/null) set -- -if [ -n "$frag" ] && [ -f "$dir/$frag" ]; then set -- -require "$dir/$frag" +while IFS="$(printf '\t')" read -r pp port; do + [ "$pp" = "$name" ] && [ -n "$port" ] && set -- "$@" -allow "$port" +done < Date: Sun, 25 Oct 2026 14:00:03 -0400 Subject: [PATCH 2/6] fix(lint): backslash-split URL continuation, flow-item reset, --json probe and parity nits (D-0615) The password-URL lint treats the line after a backslash-terminated URL line as a continuation whatever its word count, ignores a trailing comment and ends the URL at a flow-sequence item boundary. The infra and k8s --help --json probe strips only ParseBool-valid --json forms. parity-envelope.py reads only None as an empty list and checks error.message and the 'infra server ' command. --- free/infra/cmd/help_json.go | 19 +++++- free/infra/cmd/help_json_test.go | 19 ++++++ free/infra/scripts/parity-envelope.py | 28 ++++++-- free/infra/scripts/test_parity_envelope.py | 64 +++++++++++++++++++ free/k8s/cmd/help_json.go | 19 +++++- free/k8s/cmd/help_json_test.go | 18 ++++++ .../tests/test-lint-compose-password-urls.sh | 17 +++++ scripts/lint-compose-password-urls.sh | 28 +++++--- 8 files changed, 195 insertions(+), 17 deletions(-) create mode 100644 free/infra/scripts/test_parity_envelope.py diff --git a/free/infra/cmd/help_json.go b/free/infra/cmd/help_json.go index ec5e44c26..ac587ff10 100644 --- a/free/infra/cmd/help_json.go +++ b/free/infra/cmd/help_json.go @@ -28,6 +28,7 @@ import ( "bytes" "fmt" "io" + "strconv" "strings" "github.com/nself-org/cli/sdk/go/v2/output" @@ -57,7 +58,7 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han } rest := make([]string, 0, len(args)) for _, a := range args { - if a == "--json" || strings.HasPrefix(a, "--json=") { + if isJSONToken(a) { continue } rest = append(rest, a) @@ -98,6 +99,22 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han return true, 0 } +// isJSONToken reports whether a is a spelling of --json that wantsJSON accepts: +// the bare flag or --json= with a value strconv.ParseBool takes. Anything +// else (--json=maybe) stays in the probe's arguments, so the flag parse fails +// the way the normal run's parse does and the request is left to that path. +func isJSONToken(a string) bool { + if a == "--json" { + return true + } + v, ok := strings.CutPrefix(a, "--json=") + if !ok { + return false + } + _, err := strconv.ParseBool(v) + return err == nil +} + // resetFlags puts every flag of c back to its default, so a parse done only to // look for --help leaves no trace. func resetFlags(c *cobra.Command) { diff --git a/free/infra/cmd/help_json_test.go b/free/infra/cmd/help_json_test.go index 94ede3bfc..275998f18 100644 --- a/free/infra/cmd/help_json_test.go +++ b/free/infra/cmd/help_json_test.go @@ -170,3 +170,22 @@ func TestHelpJSONLeavesFlagsClean(t *testing.T) { t.Errorf("name leaked: %q changed=%v", v, f.Lookup("name").Changed) } } + +// TestHelpJSONOnlyStripsValidJSONForms: the probe drops --json spellings pflag accepts for a +// bool and keeps anything else, so `--json=maybe` makes the flag parse fail and the normal path +// (which refuses it) runs, instead of the probe answering help for an invalid request. +func TestHelpJSONOnlyStripsValidJSONForms(t *testing.T) { + rootOnce.Do(configureRoot) + for _, tok := range []string{"--json", "--json=true", "--json=1", "--json=T"} { + var out bytes.Buffer + if handled, code := helpJSON(infraCmd, []string{"plan", "--help", tok}, &out, &bytes.Buffer{}); !handled || code != 0 { + t.Errorf("plan --help %s: handled=%v code=%d, want handled", tok, handled, code) + } + } + for _, tok := range []string{"--json=maybe", "--json=", "--json=yes"} { + var out bytes.Buffer + if handled, _ := helpJSON(infraCmd, []string{"plan", "--help", "--json", tok}, &out, &bytes.Buffer{}); handled || out.Len() != 0 { + t.Errorf("plan --help --json %s: handled=%v stdout=%q, want unhandled and silent", tok, handled, out.String()) + } + } +} diff --git a/free/infra/scripts/parity-envelope.py b/free/infra/scripts/parity-envelope.py index 27184cf7e..81766bef1 100644 --- a/free/infra/scripts/parity-envelope.py +++ b/free/infra/scripts/parity-envelope.py @@ -5,8 +5,8 @@ prints its legacy JSON (or nothing on stdout when it fails). A byte diff of stdout can no longer be the check. This script checks the head's contract and the documented SURF-29 mapping instead. Usage: parity-envelope.py (sub = provision|list|resize|destroy) -Checks: 1. head stdout is exactly one JSON document: a v1 envelope (schema_version "1", a command string, exactly one - of data or error). Nothing before or after it. +Checks: 1. head stdout is exactly one JSON document: a v1 envelope (schema_version "1", a command equal to "infra server ", + exactly one of data or error). Nothing before or after it. 2. status: core exit 0 <=> envelope ok and head exit 0. Core exit 1 <=> error envelope whose exit_code equals the head exit, the exit_code matches its class (user 1, infra 2, auth 3, destructive_blocked 4, see cmd/errcodes.go) and is not 0. @@ -17,7 +17,9 @@ resize core object == data.action (data also holds server_id, target_type) destroy SnapshotID/RetainedIPs/ReleasedIPs == snapshot_id/retained_ips/released_ips (null == []; data also holds server_id) + Only None (core's null list) is read as an empty list; false, 0, "" and {} are differences. The error text is not compared: core prints "Error: ..." on stderr, the head's envelope message is its own. + Its type is: error.message must be a string. Output: one line per problem; exit 0 when none, 1 otherwise. """ import json @@ -26,6 +28,11 @@ CLASS_EXIT = {"user": 1, "infra": 2, "auth": 3, "destructive_blocked": 4} +def none_as_empty(v): + """core prints a nil slice as null; that is the only value read as an empty list.""" + return [] if v is None else v + + def canon(v): return json.dumps(v, sort_keys=True, separators=(",", ":")) @@ -40,22 +47,29 @@ def load_one(path): return doc -def check_envelope(doc): +def check_envelope(doc, sub): errs = [] if not isinstance(doc, dict): return ["not a JSON object"] if doc.get("schema_version") != "1": errs.append("schema_version is %r, want '1'" % (doc.get("schema_version"),)) + want_cmd = "infra server " + sub if not isinstance(doc.get("command"), str) or not doc.get("command"): errs.append("command missing") + elif doc["command"] != want_cmd: + errs.append("command is %r, want %r" % (doc["command"], want_cmd)) if ("data" in doc) == ("error" in doc): errs.append("want exactly one of data or error") err = doc.get("error") if err is not None: + if not isinstance(err, dict): + return errs + ["error is not an object"] if err.get("class") not in CLASS_EXIT or CLASS_EXIT[err.get("class")] != err.get("exit_code"): errs.append("exit_code %r does not match class %r" % (err.get("exit_code"), err.get("class"))) if not isinstance(err.get("code"), str): errs.append("error.code missing") + if not isinstance(err.get("message"), str): + errs.append("error.message is %r, want a string" % (err.get("message"),)) return errs @@ -86,9 +100,9 @@ def compare_data(sub, core, data): errs.append("core action != data.action") elif sub == "destroy": if need_keys(data, ["server_id", "snapshot_id", "retained_ips", "released_ips"], errs): - want = {"snapshot_id": core.get("SnapshotID"), "retained_ips": core.get("RetainedIPs") or [], - "released_ips": core.get("ReleasedIPs") or []} - got = {k: (data[k] or [] if k.endswith("_ips") else data[k]) for k in want} + want = {"snapshot_id": core.get("SnapshotID"), "retained_ips": none_as_empty(core.get("RetainedIPs")), + "released_ips": none_as_empty(core.get("ReleasedIPs"))} + got = {k: (none_as_empty(data[k]) if k.endswith("_ips") else data[k]) for k in want} if canon(want) != canon(got): errs.append("core destroy result != data: core=%s data=%s" % (canon(want), canon(got))) else: @@ -103,7 +117,7 @@ def main(argv): except ValueError as e: print(" head stdout: %s" % e) return 1 - errs = check_envelope(doc) + errs = check_envelope(doc, sub) if core_rc not in (0, 1): errs.append("core exit %d, want 0 or 1" % core_rc) ok = "error" not in doc diff --git a/free/infra/scripts/test_parity_envelope.py b/free/infra/scripts/test_parity_envelope.py new file mode 100644 index 000000000..351eb3473 --- /dev/null +++ b/free/infra/scripts/test_parity_envelope.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +"""Tests for free/infra/scripts/parity-envelope.py (D-0615). + +Run: python3 free/infra/scripts/test_parity_envelope.py (or pytest). No Docker, no network. +""" +import importlib.util +import os +import unittest + +HERE = os.path.dirname(os.path.abspath(__file__)) +_spec = importlib.util.spec_from_file_location("pe", os.path.join(HERE, "parity-envelope.py")) +pe = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(pe) + + +def err_env(command="infra server destroy", message="refused", **over): + e = {"class": "user", "exit_code": 1, "code": "E401", "message": message} + e.update(over) + return {"schema_version": "1", "command": command, "error": e} + + +class CheckEnvelope(unittest.TestCase): + def test_good_error_envelope(self): + self.assertEqual(pe.check_envelope(err_env(), "destroy"), []) + + def test_command_must_be_infra_server_sub(self): + for cmd in ("infra", "infra plan", "infra server list", "server destroy"): + self.assertTrue(pe.check_envelope(err_env(command=cmd), "destroy"), cmd) + + def test_message_must_be_a_string(self): + for bad in (None, 7, ["x"], {"a": 1}): + errs = pe.check_envelope(err_env(message=bad), "destroy") + self.assertTrue(any("error.message" in e for e in errs), bad) + e = err_env() + del e["error"]["message"] + self.assertTrue(any("error.message" in x for x in pe.check_envelope(e, "destroy"))) + + def test_error_that_is_not_an_object(self): + doc = {"schema_version": "1", "command": "infra server destroy", "error": "boom"} + self.assertTrue(pe.check_envelope(doc, "destroy")) + + +class DestroyData(unittest.TestCase): + DATA = {"server_id": 1, "snapshot_id": None, "retained_ips": [], "released_ips": []} + + def compare(self, core, **over): + data = dict(self.DATA) + data.update(over) + return pe.compare_data("destroy", core, data) + + def test_null_lists_equal_empty_lists(self): + self.assertEqual(self.compare({"SnapshotID": None, "RetainedIPs": None, "ReleasedIPs": None}), []) + self.assertEqual(self.compare({"SnapshotID": None, "RetainedIPs": [], "ReleasedIPs": []}, retained_ips=None), []) + + def test_other_empty_values_are_not_an_empty_list(self): + for falsy in (False, 0, "", {}): + core = {"SnapshotID": None, "RetainedIPs": falsy, "ReleasedIPs": []} + self.assertTrue(self.compare(core), "core %r" % (falsy,)) + core = {"SnapshotID": None, "RetainedIPs": [], "ReleasedIPs": []} + self.assertTrue(self.compare(core, released_ips=falsy), "head %r" % (falsy,)) + + +if __name__ == "__main__": + unittest.main() diff --git a/free/k8s/cmd/help_json.go b/free/k8s/cmd/help_json.go index 75df0c899..b4c58a684 100644 --- a/free/k8s/cmd/help_json.go +++ b/free/k8s/cmd/help_json.go @@ -28,6 +28,7 @@ import ( "bytes" "fmt" "io" + "strconv" "strings" "github.com/nself-org/cli/sdk/go/v2/output" @@ -57,7 +58,7 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han } rest := make([]string, 0, len(args)) for _, a := range args { - if a == "--json" || strings.HasPrefix(a, "--json=") { + if isJSONToken(a) { continue } rest = append(rest, a) @@ -101,6 +102,22 @@ func helpJSON(root *cobra.Command, args []string, stdout, stderr io.Writer) (han return true, 0 } +// isJSONToken reports whether a is a spelling of --json that wantsJSON accepts: +// the bare flag or --json= with a value strconv.ParseBool takes. Anything +// else (--json=maybe) stays in the probe's arguments, so the flag parse fails +// the way the normal run's parse does and the request is left to that path. +func isJSONToken(a string) bool { + if a == "--json" { + return true + } + v, ok := strings.CutPrefix(a, "--json=") + if !ok { + return false + } + _, err := strconv.ParseBool(v) + return err == nil +} + // resetFlags puts every flag of c back to its default, so a parse done only to // look for --help leaves no trace. func resetFlags(c *cobra.Command) { diff --git a/free/k8s/cmd/help_json_test.go b/free/k8s/cmd/help_json_test.go index b7af0abd5..37a2f2990 100644 --- a/free/k8s/cmd/help_json_test.go +++ b/free/k8s/cmd/help_json_test.go @@ -111,3 +111,21 @@ func TestHelpFlagValueIsNotHelp(t *testing.T) { t.Fatalf("control: handled=%v code=%d RunE calls=%d", handled, code, *n) } } + +// TestHelpJSONOnlyStripsValidJSONForms: the probe drops --json spellings pflag accepts for a +// bool and keeps anything else, so `--json=maybe` makes the flag parse fail and the normal path +// (which refuses it) runs, instead of the probe answering help for an invalid request. +func TestHelpJSONOnlyStripsValidJSONForms(t *testing.T) { + for _, tok := range []string{"--json", "--json=true", "--json=1", "--json=T"} { + var out bytes.Buffer + if handled, code := helpJSON(rootCmd, []string{"install", "--help", tok}, &out, &bytes.Buffer{}); !handled || code != 0 { + t.Errorf("install --help %s: handled=%v code=%d, want handled", tok, handled, code) + } + } + for _, tok := range []string{"--json=maybe", "--json=", "--json=yes"} { + var out bytes.Buffer + if handled, _ := helpJSON(rootCmd, []string{"install", "--help", "--json", tok}, &out, &bytes.Buffer{}); handled || out.Len() != 0 { + t.Errorf("install --help --json %s: handled=%v stdout=%q, want unhandled and silent", tok, handled, out.String()) + } + } +} diff --git a/scripts/ci/tests/test-lint-compose-password-urls.sh b/scripts/ci/tests/test-lint-compose-password-urls.sh index 881b83035..fd4d1946c 100755 --- a/scripts/ci/tests/test-lint-compose-password-urls.sh +++ b/scripts/ci/tests/test-lint-compose-password-urls.sh @@ -46,6 +46,15 @@ check name-pwd reject ' - E=postgresql://u:${DB_PWD}@postgres/db' check name-secret reject ' - B=amqp://u:${RABBIT_SECRET}@mq:5672' check name-token-unbraced reject ' - C=redis://:$REDIS_AUTH_TOKEN@redis:6379' check name-lowercase-default reject ' - D=postgres://u:${db_pass:-x}@pg/db' +# D-0615: a `\`-split URL whose continuation line has a trailing comment or more than one word. +check split-backslash-comment reject ' URL: "https://api.example.com/v1?token=\' \ + ' ${API_TOKEN}" # secret from the vault' +check split-backslash-two-words reject ' URL: "https://api.example.com/v1?token=\' \ + ' ${API_TOKEN}&mode=fast tail"' +check split-backslash-chain reject ' URL: "https://\' ' api.example.com/v1?a=1\' \ + ' &token=${API_TOKEN}" # last' +check split-backslash-comment-head reject ' URL: "https://api.example.com/v1?token=\ # head' \ + ' ${API_TOKEN}"' check nested-form accept \ ' - DATABASE_URL=postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD_URLENC:-${POSTGRES_PASSWORD}}@postgres:5432/${POSTGRES_DB}' \ ' - REDIS_URL=redis://:${REDIS_PASSWORD_URLENC:-${REDIS_PASSWORD}}@redis:6379 http://a@b' @@ -55,6 +64,14 @@ check secrets-outside-urls accept ' - POSTGRES_PASSWORD=${POSTGRES_PASSWORD ' - KEYCLOAK_URL=http://${KEYCLOAK_HOST}:8080' \ ' healthcheck:' ' test:' ' - CMD-SHELL' ' - |' \ ' wget -q http://127.0.0.1:3107/health' ' redis-cli -a ${REDIS_PASSWORD} ping' +check split-after-braced-default reject ' D: >-' ' postgresql://u:${DB_HOST:-${H}}' ' ${POSTGRES_PASSWORD}' ' @pg/db' +# A flow sequence closes its item at the comma: the next item is not part of the URL; a space before the backslash ends the URL. +check flow-sequence-items accept ' healthcheck:' \ + ' test: ["CMD", "curl", "http://127.0.0.1:3107/health",' ' "${REDIS_PASSWORD}"]' +check flow-sequence-item-per-line accept ' healthcheck:' ' test: [' ' "curl http://127.0.0.1:3107/health",' \ + ' "${REDIS_PASSWORD}",' ' ]' +check shell-continuation-after-url accept ' command:' ' - |' ' curl -sf http://127.0.0.1:3107/health \' \ + ' -H "X: ${API_TOKEN}"' if [ "$fail" -ne 0 ]; then echo "test-lint-compose-password-urls: FAILED" >&2; exit 1; fi echo "test-lint-compose-password-urls: OK" diff --git a/scripts/lint-compose-password-urls.sh b/scripts/lint-compose-password-urls.sh index 903dfcc54..d26db3cef 100755 --- a/scripts/lint-compose-password-urls.sh +++ b/scripts/lint-compose-password-urls.sh @@ -16,9 +16,11 @@ # secret variable anywhere: userinfo, path or query, # 2. a secret variable is followed by "@" in the same word (the userinfo # tail of a URL whose "://" sits on the line above), or -# 3. it is a one-word line below a "://" line and indented deeper than the -# key or list item holding that scalar, i.e. a URL split across three -# or more lines. +# 3. it continues a URL split across lines: the line after a "://" line +# that ends in a backslash stuck to URL text (any word count, a trailing +# " #..." comment ignored), or a one-word line below a "://" line and +# indented deeper than the key or list item holding that scalar. A +# flow-sequence item boundary (a comma, a closing bracket) ends the URL. # Inputs: fragment paths as arguments; default free/*/docker-compose.plugin.yml. # Outputs: one "file:line: ..." line per offence on stderr; exit 1 if any. # Constraints: bash 3.2 compatible; sed/grep/awk only; never prints a password. @@ -40,15 +42,25 @@ for f in $files; do # Lines naming a secret variable, comments dropped, prefixed "N:". hits="$(grep -niE "$secret" "$f" | grep -vE '^[0-9]+:[[:space:]]*#' || true)" [ -n "$hits" ] || continue - # Line numbers of one-word continuation lines inside a multi-line URL. + # Line numbers of continuation lines inside a multi-line URL: the line after one that ends in a backslash stuck to + # URL text (any word count), and a one-word line below a "://" line. A trailing " #..." comment is not text of + # the line; a flow-sequence item boundary (a comma, or the closing bracket) ends the URL. cont=" $(awk ' /^[[:space:]]*(#|$)/ { next } - { match($0, /^[[:space:]]*/); ind = RLENGTH - key = ($0 ~ /^[[:space:]]*(- |[A-Za-z0-9_.-]+:([[:space:]]|$))/) } + { line = $0; sub(/[[:space:]]+#.*$/, "", line) + match(line, /^[[:space:]]*/); ind = RLENGTH + key = (line ~ /^[[:space:]]*(- |[A-Za-z0-9_.-]+:([[:space:]]|$))/) + n = split(line, w, " ") } url && ind <= kind { url = 0 } - url && NF == 1 { printf "%d ", NR } + chain || (url && n == 1) { printf "%d ", NR } !url && key { kind = ind } - /:\/\// { url = 1 } + line ~ /:\/\// { url = 1 } + { bs = (line ~ /[^[:space:]]\\$/) + chain = bs && (chain || w[n] ~ /:\/\//) + q = line; gsub(/"[^"]*"/, "", q); gsub(/\047[^\047]*\047/, "", q) + t = q; o = gsub(/\[/, "", t); t = q; c = gsub(/\]/, "", t) + was = depth; depth += o - c; if (depth < 0) depth = 0 + if ((depth > 0 && line ~ /,$/) || (was > 0 && depth == 0)) url = 0 } ' "$f")" while IFS= read -r line; do [ -n "$line" ] || continue From 41c16aa463ee5fcdcbcabaef2d3a0ec7f7f73cb9 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 25 Oct 2026 14:00:04 -0400 Subject: [PATCH 3/6] fix(catalog): find the bundles sibling of the main checkout from a worktree (D-1169) --- .../tests/test-plugin-counts-bundles-dir.sh | 47 +++++++++++++++++++ scripts/plugin-counts.sh | 2 +- 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100755 scripts/ci/tests/test-plugin-counts-bundles-dir.sh diff --git a/scripts/ci/tests/test-plugin-counts-bundles-dir.sh b/scripts/ci/tests/test-plugin-counts-bundles-dir.sh new file mode 100755 index 000000000..e6d73e29b --- /dev/null +++ b/scripts/ci/tests/test-plugin-counts-bundles-dir.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# test-plugin-counts-bundles-dir.sh +# +# Purpose: regression cases for the bundles checkout autodetect of scripts/plugin-counts.sh (D-1169). From a git worktree +# of the plugins repo the script looks beside the MAIN checkout: both
/../bundles and
/../plugins-pro must +# be found (the bundles sibling was missing). +# Inputs: none. Builds a throwaway tree: /nself/plugins (a repo holding the script), a worktree of it under +# .claude/worktrees, and an empty git repo as the bundles sibling. +# Outputs: "ok:"/"FAIL:" per case, final summary; exit 1 on any failure. +# Constraints: bash 3.2 compatible; no docker, no network; the sibling holds no commit, so the script stops at its +# "does not hold " precondition (exit 3) and the message names the checkout it detected. +set -u + +here="$(cd "$(dirname "$0")" && pwd)" +repo="$(cd "$here/../../.." && pwd)" +tmp="$(mktemp -d "${TMPDIR:-/tmp}/plugin-counts-bd.XXXXXX")" || exit 1 +trap 'rm -rf "$tmp"' EXIT +fail=0 + +main="$tmp/nself/plugins" +mkdir -p "$main/scripts" "$tmp/nself/.claude" +cp "$repo/scripts/plugin-counts.sh" "$repo/scripts/bundles.ref" "$main/scripts/" +echo '{}' > "$main/releases.json" +g() { git -c user.name=t -c user.email=t@example.invalid -c commit.gpgsign=false "$@"; } +g -C "$main" init -q . && g -C "$main" add scripts releases.json && g -C "$main" commit -q -m seed || { echo "FAIL: cannot build the fixture repo" >&2; exit 1; } +g -C "$main" worktree add -q "$main/.claude/worktrees/wt" -b wt || { echo "FAIL: cannot add the worktree" >&2; exit 1; } +wt="$main/.claude/worktrees/wt" + +# run_case NAME SIBLING WANT_RC: run --check from the worktree with only SIBLING beside the main checkout. +run_case() { + name="$1" sibling="$2" want="$3" + rm -rf "$tmp/nself/bundles" "$tmp/nself/plugins-pro" + [ -z "$sibling" ] || g init -q "$tmp/nself/$sibling" + out="$(env -u BUNDLES_DIR -u NSELF bash "$wt/scripts/plugin-counts.sh" --check 2>&1)"; rc=$? + case "$want" in + none) ok=0; [ "$rc" -eq 3 ] && printf '%s\n' "$out" | grep -q 'no bundles checkout' && ok=1 ;; + *) ok=0; [ "$rc" -eq 3 ] && printf '%s\n' "$out" | grep -q "/\.\./\.\./$want does not hold" && ok=1 ;; + esac + if [ "$ok" -eq 1 ]; then echo "ok: $name"; else echo "FAIL: $name (rc $rc): $out" >&2; fail=1; fi +} + +run_case worktree-finds-bundles-sibling bundles bundles +run_case worktree-finds-plugins-pro-sibling plugins-pro plugins-pro +run_case worktree-without-sibling-says-so "" none + +if [ "$fail" -ne 0 ]; then echo "test-plugin-counts-bundles-dir: FAILED" >&2; exit 1; fi +echo "test-plugin-counts-bundles-dir: OK" diff --git a/scripts/plugin-counts.sh b/scripts/plugin-counts.sh index 4a930e6ed..247317a91 100755 --- a/scripts/plugin-counts.sh +++ b/scripts/plugin-counts.sh @@ -76,7 +76,7 @@ else BUNDLES_DIR="${BUNDLES_DIR:-}" COMMON="$(git -C "$ROOT" rev-parse --git-common-dir 2>/dev/null || echo /nonexistent)"; [[ "$COMMON" = /* ]] || COMMON="$ROOT/$COMMON" if [ -z "$BUNDLES_DIR" ]; then - for d in "$ROOT/../bundles" "$ROOT/../plugins-pro" "${NSELF:-/nonexistent}/plugins-pro" "${NSELF:-/nonexistent}/bundles" "$COMMON/../../plugins-pro"; do [ -d "$d/.git" ] || [ -f "$d/.git" ] && { BUNDLES_DIR="$d"; break; }; done + for d in "$ROOT/../bundles" "$ROOT/../plugins-pro" "${NSELF:-/nonexistent}/plugins-pro" "${NSELF:-/nonexistent}/bundles" "$COMMON/../../bundles" "$COMMON/../../plugins-pro"; do [ -d "$d/.git" ] || [ -f "$d/.git" ] && { BUNDLES_DIR="$d"; break; }; done fi [ -n "$BUNDLES_DIR" ] || { echo "plugin-counts: no bundles checkout (set BUNDLES_DIR, or use --check --free-only)" >&2; exit 3; } if ! git -C "$BUNDLES_DIR" cat-file -e "$BUNDLES_REF^{commit}" 2>/dev/null; then From c02fe88710df5d8a2e946788c1c47581512f07ae Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 25 Oct 2026 14:00:04 -0400 Subject: [PATCH 4/6] fix(release): rehearse.sh refuses a trailing --tag or --out instead of hanging (D-1107) --- scripts/release/rehearse-args-test.sh | 32 +++++++++++++++++++++++++++ scripts/release/rehearse.sh | 12 ++++++---- 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100755 scripts/release/rehearse-args-test.sh diff --git a/scripts/release/rehearse-args-test.sh b/scripts/release/rehearse-args-test.sh new file mode 100755 index 000000000..58dde2cb1 --- /dev/null +++ b/scripts/release/rehearse-args-test.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# rehearse-args-test.sh: scripts/release/rehearse.sh must refuse a --tag or --out with no value (exit 2, a message naming the +# flag) instead of looping forever on it (D-1107: the failed `shift 2` was ignored). Each case runs under a watchdog, so a +# regression shows as a HUNG failure, not a stuck test. No key, no tarball and no network is touched: every case is refused +# during argument parsing, before the script reads anything else. +# Run from any directory: bash scripts/release/rehearse-args-test.sh +set -u +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +T="$(mktemp -d "${TMPDIR:-/tmp}/rehearse-args.XXXXXX")"; trap 'rm -rf "$T"' EXIT +fails=0 + +# case_rc