From 7388bc491c4ddd98da8a5587e9390b123315036f Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 07:48:48 -0700 Subject: [PATCH] fix(miner): keep AMS policy operator-local --- .gittensory-ams.yml.example | 17 +---- packages/gittensory-engine/README.md | 10 +-- .../gittensory-engine/src/ams-policy-spec.ts | 14 +--- packages/gittensory-miner/lib/ams-policy.d.ts | 7 +- packages/gittensory-miner/lib/ams-policy.js | 69 ++++------------- test/unit/miner-ams-policy.test.ts | 75 ++++--------------- 6 files changed, 46 insertions(+), 146 deletions(-) diff --git a/.gittensory-ams.yml.example b/.gittensory-ams.yml.example index 9743c1f1e..8b23206b6 100644 --- a/.gittensory-ams.yml.example +++ b/.gittensory-ams.yml.example @@ -10,25 +10,16 @@ # field here ever lets a target repo loosen what an operator's agent is # willing to do -- see below for why. # -# TWO SCOPES, mirroring `.gittensory.yml`'s own established self-host -# precedent: a target repo MAY drop this file in as a proposed DEFAULT policy -# for operators who haven't set their own (e.g. "please use a strict slop -# threshold mining me"). But the OPERATOR's own local copy of this file (in -# their gittensory-miner config dir, not this repo) -- when present -- FULLY -# REPLACES whatever this repo's file says. Never a field-by-field merge. An -# operator's explicit choice always wins; a repo's file can only ever propose -# a fallback default for an unconfigured operator, never override an -# operator's own decision. +# OPERATOR-LOCAL ONLY: the miner reads this file from the operator's local +# gittensory-miner config dir. It intentionally does not fetch a target repo's +# checked-in AMS policy, because untrusted repo content must not loosen +# operator-side budget, turn, slop, or submission controls. # # Every field is OPTIONAL and has a safe, deny-by-default default (shown as # "Default: X" below). The file is parsed tolerantly: an unknown key is # ignored, and a single malformed field falls back to its default with a # warning. # -# Discovery order (first match wins): -# .gittensory-ams.yml -> .github/gittensory-ams.yml -# -> .gittensory-ams.json -> .github/gittensory-ams.json -# # Copy to `.gittensory-ams.yml` and edit. YAML or JSON are both accepted. # Whether a real attempt is allowed to actually submit (open a PR), or only diff --git a/packages/gittensory-engine/README.md b/packages/gittensory-engine/README.md index a3ce36726..a8ed688c0 100644 --- a/packages/gittensory-engine/README.md +++ b/packages/gittensory-engine/README.md @@ -656,11 +656,11 @@ submission mode (computes real decisions but never actually submits) and a `"low `parseAmsPolicySpec(raw)` / `parseAmsPolicySpecContent(content)` are the same tolerant-parser pair shape as `MinerGoalSpec`'s — never throw, return `{ present, spec, warnings }`. -Unlike `MinerGoalSpec`, this package does not resolve `.gittensory-ams.yml`'s two-scope precedence itself (this -package is IO-free) — `packages/gittensory-miner/lib/ams-policy.js`'s `resolveAmsPolicy` is the real caller, -mirroring `.gittensory.yml`'s own established self-host precedent: the operator's own local file, when present, -FULLY REPLACES the repo's proposed file (never a field-by-field merge) — the repo's file is only ever a fallback -default for an operator who hasn't set their own local policy. See `.gittensory-ams.yml.example`. +Unlike `MinerGoalSpec`, this package does not resolve `.gittensory-ams.yml` from the filesystem itself (this +package is IO-free) — `packages/gittensory-miner/lib/ams-policy.js`'s `resolveAmsPolicy` is the real caller. +That resolver reads only the operator's local policy and otherwise uses safe defaults; it intentionally does not +fetch a target repo's checked-in AMS policy, because untrusted repo content must not loosen operator-side budget, +turn, slop, or submission controls. See `.gittensory-ams.yml.example`. ## Repo map builder diff --git a/packages/gittensory-engine/src/ams-policy-spec.ts b/packages/gittensory-engine/src/ams-policy-spec.ts index 61b89c194..78d5cb73c 100644 --- a/packages/gittensory-engine/src/ams-policy-spec.ts +++ b/packages/gittensory-engine/src/ams-policy-spec.ts @@ -12,16 +12,10 @@ import { DEFAULT_PORTFOLIO_CONVERGENCE_THRESHOLDS, type PortfolioConvergenceThre // its own risk tolerance against that exact repo. So this type is intentionally free of any field a target // repo could use to loosen what an operator's agent is willing to do. // -// Two-scope resolution, mirroring `.gittensory.yml`'s own established self-host precedent (see -// `src/selfhost/private-config.ts`'s `makeLocalManifestReader`, whose own doc comment is explicit: a -// self-host operator's local file "takes priority over -- and fully REPLACES -- the public .gittensory.yml"): -// 1. A repo-scoped `.gittensory-ams.yml` MAY exist in the target repo, proposing a DEFAULT execution policy -// for operators who haven't set their own (e.g. "please use a strict slop threshold mining me"). -// 2. The operator's own local `.gittensory-ams.yml` (in their `gittensory-miner` config dir), when present, -// FULLY REPLACES the repo's proposed file -- never a field-by-field merge. An operator's explicit choice -// always wins; the repo's file is only ever a fallback default for an unconfigured operator. -// The actual two-scope fetch+resolve lives in packages/gittensory-miner/lib/ams-policy.js (this package is -// IO-free, same discipline as miner-goal-spec.ts) -- this module is the type/parser surface only. +// Resolution deliberately stays operator-local: packages/gittensory-miner/lib/ams-policy.js reads only the +// operator's own local `.gittensory-ams.yml` (in their `gittensory-miner` config dir) and otherwise uses safe +// defaults. It does not fetch a target repo's checked-in file, because that would let untrusted repo content +// loosen operator-side budget, turn, slop, or submission controls. /** Whether a real attempt is allowed to actually submit (open a PR), or only compute + log its decision. * Mirrors `src/settings/autonomy.ts`'s deny-by-default dial: "observe" still runs every real signal/decision, diff --git a/packages/gittensory-miner/lib/ams-policy.d.ts b/packages/gittensory-miner/lib/ams-policy.d.ts index 3be9c27d4..de0fb4f26 100644 --- a/packages/gittensory-miner/lib/ams-policy.d.ts +++ b/packages/gittensory-miner/lib/ams-policy.d.ts @@ -1,9 +1,7 @@ import type { AmsPolicySpec } from "@jsonbored/gittensory-engine"; -import type { SelfReviewContextFetch } from "./self-review-context.js"; - export function resolveAmsPolicyConfigPath(env?: Record): string; -export type AmsPolicySource = "local" | "repo" | "default"; +export type AmsPolicySource = "local" | "default"; export type ResolvedAmsPolicy = { spec: AmsPolicySpec; @@ -14,8 +12,7 @@ export type ResolvedAmsPolicy = { export function resolveAmsPolicy( repoFullName: string, options?: { - rawContentBaseUrl?: string; - fetchImpl?: SelfReviewContextFetch; + fetchImpl?: unknown; readFileSync?: (path: string, encoding: "utf8") => string; existsSync?: (path: string) => boolean; env?: Record; diff --git a/packages/gittensory-miner/lib/ams-policy.js b/packages/gittensory-miner/lib/ams-policy.js index 4cf0bbde7..1e83fc567 100644 --- a/packages/gittensory-miner/lib/ams-policy.js +++ b/packages/gittensory-miner/lib/ams-policy.js @@ -1,20 +1,16 @@ import { existsSync, readFileSync } from "node:fs"; -import { AMS_POLICY_SPEC_FILENAMES, DEFAULT_AMS_POLICY_SPEC, parseAmsPolicySpecContent } from "@jsonbored/gittensory-engine"; +import { DEFAULT_AMS_POLICY_SPEC, parseAmsPolicySpecContent } from "@jsonbored/gittensory-engine"; import { resolveLocalStoreDbPath } from "./local-store.js"; -// Real two-scope resolver for `.gittensory-ams.yml` (#5132, Wave 3.5 follow-up). AmsPolicySpec -// (ams-policy-spec.ts, engine package) is the type/parser surface; this module is the actual fetch+resolve -// caller, mirroring `.gittensory.yml`'s own established self-host precedent (src/selfhost/private-config.ts's -// `makeLocalManifestReader`): the operator's own local file, when present, FULLY REPLACES whatever the -// target repo's own file says -- never a field-by-field merge. The repo's file is only ever consulted as a -// fallback default for an operator who hasn't set their own local policy. +// Resolver for the operator-local `.gittensory-ams.yml` (#5132, Wave 3.5 follow-up). AmsPolicySpec +// (ams-policy-spec.ts, engine package) is the type/parser surface; this module is the actual local +// read+resolve caller. // // This is deliberately NOT the same resolution shape as self-review-context.js/rejection-signal.js, which -// only ever read from the target repo: AmsPolicySpec's fields are the OPERATOR's own execution-risk policy -// (see ams-policy-spec.ts's own header for why a target repo must never get final say over that). +// read from the target repo: AmsPolicySpec's fields are the OPERATOR's own execution-risk policy, so an +// untrusted target repo must never get final say over them. const AMS_POLICY_FILENAME = ".gittensory-ams.yml"; -const DEFAULT_RAW_CONTENT_BASE_URL = "https://raw.githubusercontent.com"; /** Resolve the operator's local AMS policy file path: explicit env var > `GITTENSORY_MINER_CONFIG_DIR` > * `XDG_CONFIG_HOME`/`~/.config`, mirroring every other local-store path in this package. */ @@ -24,22 +20,12 @@ export function resolveAmsPolicyConfigPath(env = process.env) { function normalizeOptions(options = {}) { return { - rawContentBaseUrl: - typeof options.rawContentBaseUrl === "string" && options.rawContentBaseUrl.trim() ? options.rawContentBaseUrl.trim() : DEFAULT_RAW_CONTENT_BASE_URL, - fetchImpl: options.fetchImpl ?? fetch, readFileSync: options.readFileSync ?? readFileSync, existsSync: options.existsSync ?? existsSync, env: options.env ?? process.env, }; } -function parseRepoFullName(repoFullName) { - if (typeof repoFullName !== "string") return null; - const [owner, repo, extra] = repoFullName.split("/"); - if (!owner || !repo || extra !== undefined) return null; - return { owner, repo }; -} - /** Read the operator's own local `.gittensory-ams.yml`, if one exists. Never throws: an unreadable file is * treated the same as an absent one, falling through to the next resolution layer. */ function readLocalAmsPolicyContent(resolved) { @@ -52,40 +38,24 @@ function readLocalAmsPolicyContent(resolved) { } } -/** Fetch the target repo's own proposed `.gittensory-ams.yml`, trying each candidate path in order (first - * 200 OK wins), mirroring self-review-context.js's `fetchManifestContent`. */ -async function fetchRepoAmsPolicyContent(target, resolved) { - for (const path of AMS_POLICY_SPEC_FILENAMES) { - const url = `${resolved.rawContentBaseUrl}/${encodeURIComponent(target.owner)}/${encodeURIComponent(target.repo)}/HEAD/${path}`; - try { - const response = await resolved.fetchImpl(url, { method: "GET", headers: { accept: "application/json", "user-agent": "loopover-miner" } }); - if (response.ok) { - const text = await response.text(); - if (typeof text === "string") return text; - } - } catch { - // Try the next candidate path. - } - } - return null; -} - /** - * Resolve the real, effective AMS execution policy for one attempt against `repoFullName`: the operator's - * own local `.gittensory-ams.yml` when present (source: "local"), else the target repo's own proposed file - * when present (source: "repo"), else the engine's safe defaults (source: "default"). Never throws -- an - * unreadable/malformed file at either layer degrades to the next layer or the safe defaults, same discipline - * as every other tolerant parser in this pipeline. + * Resolve the real, effective AMS execution policy for one attempt: the operator's own local + * `.gittensory-ams.yml` when present (source: "local"), else the engine's safe defaults (source: "default"). + * Never throws -- an unreadable/malformed local file degrades through the tolerant parser to the safe + * defaults, same discipline as every other tolerant parser in this pipeline. + * + * `repoFullName` is accepted for API compatibility with callers that resolve policy per target repo, but the + * resolver intentionally does not fetch or trust target-repository AMS policy content. * * @param {string} repoFullName * @param {{ - * rawContentBaseUrl?: string, fetchImpl?: import("./self-review-context.js").SelfReviewContextFetch, * readFileSync?: (path: string, encoding: "utf8") => string, existsSync?: (path: string) => boolean, * env?: Record, * }} [options] - * @returns {Promise<{ spec: import("@jsonbored/gittensory-engine").AmsPolicySpec, source: "local"|"repo"|"default", warnings: string[] }>} + * @returns {Promise<{ spec: import("@jsonbored/gittensory-engine").AmsPolicySpec, source: "local"|"default", warnings: string[] }>} */ export async function resolveAmsPolicy(repoFullName, options = {}) { + void repoFullName; const resolved = normalizeOptions(options); const localContent = readLocalAmsPolicyContent(resolved); @@ -94,14 +64,5 @@ export async function resolveAmsPolicy(repoFullName, options = {}) { return { spec: parsed.spec, source: "local", warnings: parsed.warnings }; } - const target = parseRepoFullName(repoFullName); - if (target) { - const repoContent = await fetchRepoAmsPolicyContent(target, resolved); - if (repoContent !== null) { - const parsed = parseAmsPolicySpecContent(repoContent); - return { spec: parsed.spec, source: "repo", warnings: parsed.warnings }; - } - } - return { spec: DEFAULT_AMS_POLICY_SPEC, source: "default", warnings: [] }; } diff --git a/test/unit/miner-ams-policy.test.ts b/test/unit/miner-ams-policy.test.ts index db7b71a8d..5ec31c32b 100644 --- a/test/unit/miner-ams-policy.test.ts +++ b/test/unit/miner-ams-policy.test.ts @@ -22,26 +22,6 @@ function tempRoot() { return root; } -function textResponse(text: string | null, status = 200) { - return { - ok: status >= 200 && status < 300, - status, - json: async (): Promise => { - throw new Error("textResponse: json() is unused by ams-policy's fetch path"); - }, - text: async () => text ?? "", - }; -} - -function routedFetch(routes: Record ReturnType>) { - return async (url: string) => { - for (const [substring, respond] of Object.entries(routes)) { - if (url.includes(substring)) return respond(); - } - return textResponse(null, 404); - }; -} - describe("resolveAmsPolicyConfigPath (#5132)", () => { it("resolves from explicit env, config dir, and XDG default, in precedence order", () => { expect(resolveAmsPolicyConfigPath({ GITTENSORY_MINER_AMS_POLICY_PATH: "/custom/policy.yml" })).toBe("/custom/policy.yml"); @@ -50,36 +30,32 @@ describe("resolveAmsPolicyConfigPath (#5132)", () => { }); describe("resolveAmsPolicy (#5132)", () => { - it("returns the engine's safe defaults when neither a local file nor a repo file exists", async () => { + it("returns the engine's safe defaults when no local operator policy exists", async () => { const root = tempRoot(); - const fetchImpl = routedFetch({}); - const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); + const result = await resolveAmsPolicy("acme/widgets", { env: { GITTENSORY_MINER_CONFIG_DIR: root } }); expect(result).toEqual({ spec: DEFAULT_AMS_POLICY_SPEC, source: "default", warnings: [] }); }); - it("falls through to the repo's own .gittensory-ams.yml when no local file exists", async () => { + it("REGRESSION: ignores target-repo .gittensory-ams.yml so repos cannot loosen operator risk policy", async () => { const root = tempRoot(); - const fetchImpl = routedFetch({ - ".gittensory-ams.yml": () => textResponse("submissionMode: enforce\nslopThreshold: clean\n"), + const fetchImpl = vi.fn(async () => { + throw new Error("target repo policy must not be fetched"); }); const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); - expect(result.source).toBe("repo"); - expect(result.spec.submissionMode).toBe("enforce"); - expect(result.spec.slopThreshold).toBe("clean"); + expect(result).toEqual({ spec: DEFAULT_AMS_POLICY_SPEC, source: "default", warnings: [] }); + expect(fetchImpl).not.toHaveBeenCalled(); }); - it("REGRESSION: the operator's own local file fully REPLACES the repo's file, never merges", async () => { + it("the operator's own local file supplies the effective policy", async () => { const root = tempRoot(); - writeFileSync(join(root, ".gittensory-ams.yml"), "submissionMode: observe\n"); - // The repo's own file sets slopThreshold too -- if this leaked through via a merge, slopThreshold would - // read "clean" instead of the local file's own (unset -> default) "low". - const fetchImpl = vi.fn(routedFetch({ - ".gittensory-ams.yml": () => textResponse("submissionMode: enforce\nslopThreshold: clean\n"), - })); + writeFileSync(join(root, ".gittensory-ams.yml"), "submissionMode: observe\nslopThreshold: clean\n"); + const fetchImpl = vi.fn(async () => { + throw new Error("target repo policy must not be fetched"); + }); const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); expect(result.source).toBe("local"); expect(result.spec.submissionMode).toBe("observe"); - expect(result.spec.slopThreshold).toBe(DEFAULT_AMS_POLICY_SPEC.slopThreshold); + expect(result.spec.slopThreshold).toBe("clean"); expect(fetchImpl).not.toHaveBeenCalled(); }); @@ -89,7 +65,7 @@ describe("resolveAmsPolicy (#5132)", () => { let fetchCalls = 0; const fetchImpl = async () => { fetchCalls += 1; - return textResponse(null, 404); + throw new Error("target repo policy must not be fetched"); }; const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); expect(result.source).toBe("local"); @@ -102,7 +78,7 @@ describe("resolveAmsPolicy (#5132)", () => { let fetchCalls = 0; const fetchImpl = async () => { fetchCalls += 1; - return textResponse(null, 404); + throw new Error("target repo policy must not be fetched"); }; const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); expect(result.source).toBe("local"); @@ -111,30 +87,11 @@ describe("resolveAmsPolicy (#5132)", () => { expect(fetchCalls).toBe(0); }); - it("returns defaults for a malformed repoFullName, without ever calling fetch", async () => { + it("returns defaults for any repoFullName, without ever calling fetch", async () => { const root = tempRoot(); const fetchImpl = vi.fn(); const result = await resolveAmsPolicy("not-a-repo", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); expect(result).toEqual({ spec: DEFAULT_AMS_POLICY_SPEC, source: "default", warnings: [] }); expect(fetchImpl).not.toHaveBeenCalled(); }); - - it("returns defaults on a repo fetch network error", async () => { - const root = tempRoot(); - const fetchImpl = async () => { - throw new Error("network unreachable"); - }; - const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); - expect(result).toEqual({ spec: DEFAULT_AMS_POLICY_SPEC, source: "default", warnings: [] }); - }); - - it("tries the .github/ and .json candidate paths when the root .yml 404s", async () => { - const root = tempRoot(); - const fetchImpl = routedFetch({ - ".github/gittensory-ams.yml": () => textResponse("submissionMode: enforce\n"), - }); - const result = await resolveAmsPolicy("acme/widgets", { fetchImpl, env: { GITTENSORY_MINER_CONFIG_DIR: root } }); - expect(result.source).toBe("repo"); - expect(result.spec.submissionMode).toBe("enforce"); - }); });