diff --git a/packages/loopover-engine/src/ams-policy-spec.ts b/packages/loopover-engine/src/ams-policy-spec.ts index 59c9389b01..21cf14f27d 100644 --- a/packages/loopover-engine/src/ams-policy-spec.ts +++ b/packages/loopover-engine/src/ams-policy-spec.ts @@ -42,6 +42,42 @@ export type AmsCapLimits = { elapsedMs: number; }; +/** Curated ecosystem identifiers an operator may declare in {@link AmsNetworkAllowlist.ecosystems} -- the + * language/package-manager registries #7648 ratified as a safe default category. A closed set (not free + * text) so a typo degrades to a warning + drop, not a silently-ignored no-op. */ +export const AMS_NETWORK_ALLOWLIST_ECOSYSTEMS = ["npm", "pypi", "crates", "go", "rubygems", "packagist", "maven", "nuget"] as const; +export type AmsNetworkAllowlistEcosystem = (typeof AMS_NETWORK_ALLOWLIST_ECOSYSTEMS)[number]; + +const MAX_NETWORK_ALLOWLIST_ECOSYSTEMS = AMS_NETWORK_ALLOWLIST_ECOSYSTEMS.length; +const MAX_NETWORK_ALLOWLIST_EXTRA_HOSTS = 50; +// RFC 1123 hostname shape (labels of letters/digits/hyphens, dot-separated, no leading/trailing hyphen per +// label) -- deliberately conservative since a future enforcement implementation (#7857's still-open mechanism +// half) will feed this straight into firewall/proxy rules; garbage here would be that implementation's problem +// to sanitize a second time. +const HOSTNAME_RE = /^(?=.{1,253}$)(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)*[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?$/; + +/** Operator-declared network-egress allowlist additions (#7857, config-surface half of #7648's ratified + * design) for AMS sandboxed execution. Deliberately operator-local ONLY, mirroring this whole file's own + * scope (see the module header) -- never fetched from a target repo. #7648 ratified "the repo's declared + * language-ecosystem registries" as a default-allowlist category, but deriving that from a TARGET repo's own + * manifest is unsafe: a malicious repo could fabricate a manifest entry to smuggle an attacker-controlled + * host into its own attempt's allowlist -- exactly the kind of repo-loosens-its-own-constraints hole this + * file's whole design already guards against. The operator declares which ecosystems and any extra hosts + * their own repos legitimately need instead. + * + * INERT today: no OS-level network-egress enforcement exists yet for AMS sandboxed execution (#7857's + * mechanism half is still open, deliberately deferred separately from this config surface). This type is + * what a future enforcement implementation will read; landing it now settles the trust-boundary question + * ahead of that work instead of leaving it to be reopened once enforcement is being built. */ +export type AmsNetworkAllowlist = { + /** Ecosystem registries to allow, beyond the two categories #7648 ratified as always-on (OS package + * registries, the repo's own git remote) -- those aren't declared here since they apply unconditionally. */ + ecosystems: AmsNetworkAllowlistEcosystem[]; + /** Additional specific hostnames to allow beyond the curated ecosystem categories, e.g. a project's own + * third-party API (#7648's "requesting broader access" case). */ + extraHosts: string[]; +}; + /** Per-operator AMS execution policy parsed from `.loopover-ams.yml`. See {@link DEFAULT_AMS_POLICY_SPEC}. */ export type AmsPolicySpec = { /** Whether a real attempt may actually submit. Default: "observe" (deny-by-default). */ @@ -65,6 +101,10 @@ export type AmsPolicySpec = { * hands off), so defaulting to "observe" would silently change behavior for every operator who leaves the * field unset. Inert until the consultation issue reads it. */ selfLoopAutonomy: AutonomyLevel; + /** Operator-declared network-egress allowlist additions (#7857). Default: `{ ecosystems: [], extraHosts: [] }` + * -- no additions beyond the always-on OS-registry/git-remote defaults. INERT until #7857's OS-level + * enforcement mechanism is built; see {@link AmsNetworkAllowlist}'s own doc comment. */ + networkAllowlist: AmsNetworkAllowlist; }; /** The tolerant parser result for `.loopover-ams.yml`. Mirrors `ParsedMinerGoalSpec`'s present/warnings shape. */ @@ -86,6 +126,7 @@ export const DEFAULT_AMS_POLICY_SPEC: Readonly = Object.freeze({ maxIterations: 3, maxTurnsPerIteration: 6, selfLoopAutonomy: "auto", + networkAllowlist: Object.freeze({ ecosystems: [], extraHosts: [] }), }); const MAX_AMS_POLICY_SPEC_BYTES = 8_192; @@ -99,6 +140,10 @@ function cloneDefaultAmsPolicySpec(): AmsPolicySpec { maxIterations: DEFAULT_AMS_POLICY_SPEC.maxIterations, maxTurnsPerIteration: DEFAULT_AMS_POLICY_SPEC.maxTurnsPerIteration, selfLoopAutonomy: DEFAULT_AMS_POLICY_SPEC.selfLoopAutonomy, + networkAllowlist: { + ecosystems: [...DEFAULT_AMS_POLICY_SPEC.networkAllowlist.ecosystems], + extraHosts: [...DEFAULT_AMS_POLICY_SPEC.networkAllowlist.extraHosts], + }, }; } @@ -185,6 +230,67 @@ function normalizeConvergenceThresholds( }; } +/** Validates each entry independently and DROPS invalid ones rather than falling back to the whole list -- + * unlike this file's single-value fields (one bad value = the whole field reverts to default), a list field + * reverting entirely on one typo would silently discard every other correctly-typed entry alongside it. */ +function normalizeEcosystemList(value: unknown, fallback: AmsNetworkAllowlistEcosystem[], warnings: string[]): AmsNetworkAllowlistEcosystem[] { + // A fresh copy, not `fallback` by reference: unlike this file's number-valued fields, an array is mutable, + // so passing through the DEFAULT_AMS_POLICY_SPEC singleton's own array here would let a caller who mutates + // their OWN resolved spec's list (e.g. `.push`) silently corrupt every other caller's shared defaults too. + if (value === undefined || value === null) return [...fallback]; + if (!Array.isArray(value)) { + warnings.push('AmsPolicySpec field "networkAllowlist.ecosystems" must be an array; falling back to defaults.'); + return [...fallback]; + } + const known = new Set(AMS_NETWORK_ALLOWLIST_ECOSYSTEMS); + const result: AmsNetworkAllowlistEcosystem[] = []; + for (const entry of value.slice(0, MAX_NETWORK_ALLOWLIST_ECOSYSTEMS)) { + if (typeof entry === "string" && known.has(entry) && !result.includes(entry as AmsNetworkAllowlistEcosystem)) { + result.push(entry as AmsNetworkAllowlistEcosystem); + continue; + } + warnings.push( + `AmsPolicySpec field "networkAllowlist.ecosystems" entry ${JSON.stringify(entry)} must be one of ${AMS_NETWORK_ALLOWLIST_ECOSYSTEMS.join(", ")}; dropping it.`, + ); + } + return result; +} + +/** Same drop-invalid-entries approach as {@link normalizeEcosystemList}. Hostname shape is validated (not just + * "is this a string") because this feeds a future firewall/proxy enforcement implementation directly -- see + * {@link AmsNetworkAllowlist}'s own doc comment. */ +function normalizeExtraHosts(value: unknown, fallback: string[], warnings: string[]): string[] { + // Fresh copies throughout, same reasoning as normalizeEcosystemList's own comment above. + if (value === undefined || value === null) return [...fallback]; + if (!Array.isArray(value)) { + warnings.push('AmsPolicySpec field "networkAllowlist.extraHosts" must be an array; falling back to defaults.'); + return [...fallback]; + } + const result: string[] = []; + for (const entry of value.slice(0, MAX_NETWORK_ALLOWLIST_EXTRA_HOSTS)) { + if (typeof entry === "string" && HOSTNAME_RE.test(entry) && !result.includes(entry)) { + result.push(entry); + continue; + } + warnings.push(`AmsPolicySpec field "networkAllowlist.extraHosts" entry ${JSON.stringify(entry)} is not a valid hostname; dropping it.`); + } + return result; +} + +function normalizeNetworkAllowlist(value: unknown, fallback: AmsNetworkAllowlist, warnings: string[]): AmsNetworkAllowlist { + // Fresh array copies in the fallback object too, same reasoning as normalizeEcosystemList's own comment. + if (value === undefined || value === null) return { ecosystems: [...fallback.ecosystems], extraHosts: [...fallback.extraHosts] }; + if (typeof value !== "object" || Array.isArray(value)) { + warnings.push('AmsPolicySpec field "networkAllowlist" must be a mapping; falling back to defaults.'); + return { ecosystems: [...fallback.ecosystems], extraHosts: [...fallback.extraHosts] }; + } + const record = value as Record; + return { + ecosystems: normalizeEcosystemList(record.ecosystems, fallback.ecosystems, warnings), + extraHosts: normalizeExtraHosts(record.extraHosts, fallback.extraHosts, warnings), + }; +} + function hasConfiguredPolicyFields(spec: AmsPolicySpec): boolean { return ( spec.submissionMode !== DEFAULT_AMS_POLICY_SPEC.submissionMode || @@ -196,7 +302,12 @@ function hasConfiguredPolicyFields(spec: AmsPolicySpec): boolean { spec.convergenceThresholds.maxReenqueues !== DEFAULT_AMS_POLICY_SPEC.convergenceThresholds.maxReenqueues || spec.maxIterations !== DEFAULT_AMS_POLICY_SPEC.maxIterations || spec.maxTurnsPerIteration !== DEFAULT_AMS_POLICY_SPEC.maxTurnsPerIteration || - spec.selfLoopAutonomy !== DEFAULT_AMS_POLICY_SPEC.selfLoopAutonomy + spec.selfLoopAutonomy !== DEFAULT_AMS_POLICY_SPEC.selfLoopAutonomy || + // Default is always { ecosystems: [], extraHosts: [] } (see DEFAULT_AMS_POLICY_SPEC) -- any entry at all + // means the operator configured something, so length alone is the right "differs from default" check; + // no need to compare contents. + spec.networkAllowlist.ecosystems.length > 0 || + spec.networkAllowlist.extraHosts.length > 0 ); } @@ -244,6 +355,7 @@ export function parseAmsPolicySpec(raw: unknown): ParsedAmsPolicySpec { DEFAULT_AMS_POLICY_SPEC.selfLoopAutonomy, warnings, ), + networkAllowlist: normalizeNetworkAllowlist(record.networkAllowlist, DEFAULT_AMS_POLICY_SPEC.networkAllowlist, warnings), }; if (!hasConfiguredPolicyFields(spec)) { warnings.push("AmsPolicySpec contained no recognized non-default policy fields; falling back to safe defaults."); diff --git a/packages/loopover-engine/src/index.ts b/packages/loopover-engine/src/index.ts index 40566f3b27..9e44513759 100644 --- a/packages/loopover-engine/src/index.ts +++ b/packages/loopover-engine/src/index.ts @@ -551,7 +551,10 @@ export { parseAmsPolicySpec, parseAmsPolicySpecContent, AMS_POLICY_SPEC_FILENAMES, + AMS_NETWORK_ALLOWLIST_ECOSYSTEMS, type AmsCapLimits, + type AmsNetworkAllowlist, + type AmsNetworkAllowlistEcosystem, type AmsPolicySpec, type AmsSlopThreshold, type AmsSubmissionMode, diff --git a/packages/loopover-engine/test/ams-policy-spec-parser.test.ts b/packages/loopover-engine/test/ams-policy-spec-parser.test.ts index f7f98389a3..042d70df7c 100644 --- a/packages/loopover-engine/test/ams-policy-spec-parser.test.ts +++ b/packages/loopover-engine/test/ams-policy-spec-parser.test.ts @@ -41,6 +41,7 @@ test("parseAmsPolicySpec: valid raw config normalizes every field and keeps non- maxIterations: 5, maxTurnsPerIteration: 10, selfLoopAutonomy: "observe", + networkAllowlist: { ecosystems: ["npm"], extraHosts: ["api.example.com"] }, }); assert.equal(parsed.present, true); @@ -52,10 +53,19 @@ test("parseAmsPolicySpec: valid raw config normalizes every field and keeps non- maxIterations: 5, maxTurnsPerIteration: 10, selfLoopAutonomy: "observe", + networkAllowlist: { ecosystems: ["npm"], extraHosts: ["api.example.com"] }, }); assert.deepEqual(parsed.warnings, []); }); +test("parseAmsPolicySpec: networkAllowlist (#7857) defaults to no additions and normalizes a valid declaration", () => { + assert.deepEqual(DEFAULT_AMS_POLICY_SPEC.networkAllowlist, { ecosystems: [], extraHosts: [] }); + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm", "pypi"], extraHosts: ["api.example.com"] } }); + assert.equal(parsed.present, true); + assert.deepEqual(parsed.spec.networkAllowlist, { ecosystems: ["npm", "pypi"], extraHosts: ["api.example.com"] }); + assert.deepEqual(parsed.warnings, []); +}); + test("parseAmsPolicySpec: selfLoopAutonomy defaults to auto when omitted (#6559)", () => { // "auto" is today's implicit behavior -- a clean self-review pass already hands off unconditionally -- so // this default is what keeps an unset field from silently changing an existing operator's loop. diff --git a/test/unit/ams-policy-spec-parser.test.ts b/test/unit/ams-policy-spec-parser.test.ts index 9b750276dc..c8ffcf8ebd 100644 --- a/test/unit/ams-policy-spec-parser.test.ts +++ b/test/unit/ams-policy-spec-parser.test.ts @@ -43,6 +43,7 @@ describe("AmsPolicySpec parser (#5132)", () => { maxIterations: 5, maxTurnsPerIteration: 10, selfLoopAutonomy: "observe", + networkAllowlist: { ecosystems: ["npm", "pypi"], extraHosts: ["api.example.com"] }, }); expect(parsed.present).toBe(true); expect(parsed.spec).toEqual({ @@ -53,6 +54,7 @@ describe("AmsPolicySpec parser (#5132)", () => { maxIterations: 5, maxTurnsPerIteration: 10, selfLoopAutonomy: "observe", + networkAllowlist: { ecosystems: ["npm", "pypi"], extraHosts: ["api.example.com"] }, }); expect(parsed.warnings).toEqual([]); }); @@ -121,6 +123,137 @@ describe("AmsPolicySpec parser (#5132)", () => { }); }); + describe("networkAllowlist (#7857)", () => { + it("defaults to no additions when omitted", () => { + expect(DEFAULT_AMS_POLICY_SPEC.networkAllowlist).toEqual({ ecosystems: [], extraHosts: [] }); + expect(parseAmsPolicySpec({}).spec.networkAllowlist).toEqual({ ecosystems: [], extraHosts: [] }); + }); + + it("accepts every known ecosystem and a valid extra host, marking the spec present", () => { + const parsed = parseAmsPolicySpec({ + networkAllowlist: { + ecosystems: ["npm", "pypi", "crates", "go", "rubygems", "packagist", "maven", "nuget"], + extraHosts: ["api.example.com", "sub.deep.example.co.uk"], + }, + }); + expect(parsed.present).toBe(true); + expect(parsed.spec.networkAllowlist).toEqual({ + ecosystems: ["npm", "pypi", "crates", "go", "rubygems", "packagist", "maven", "nuget"], + extraHosts: ["api.example.com", "sub.deep.example.co.uk"], + }); + expect(parsed.warnings).toEqual([]); + }); + + it("drops unknown ecosystem entries with a warning but keeps the valid ones", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm", "yolo-registry", "pypi"] } }); + expect(parsed.spec.networkAllowlist.ecosystems).toEqual(["npm", "pypi"]); + expect(parsed.warnings.join(" ")).toMatch(/networkAllowlist\.ecosystems.*yolo-registry.*npm, pypi, crates/i); + }); + + it("drops duplicate ecosystem entries with a warning, keeping the first occurrence", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm", "npm"] } }); + expect(parsed.spec.networkAllowlist.ecosystems).toEqual(["npm"]); + expect(parsed.warnings.join(" ")).toMatch(/networkAllowlist\.ecosystems/i); + }); + + it("drops non-string ecosystem entries with a warning", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm", 42, null] } }); + expect(parsed.spec.networkAllowlist.ecosystems).toEqual(["npm"]); + expect(parsed.warnings.length).toBeGreaterThanOrEqual(2); + }); + + it("caps the raw ecosystems array at the known-ecosystem count instead of processing an unbounded list", () => { + const raw = Array.from({ length: 100 }, () => "yolo-registry"); + // maxIterations rides along so hasConfiguredPolicyFields is true for a reason unrelated to this list -- + // otherwise every entry dropping leaves the spec fully default, adding an extra "no recognized fields" + // warning that would confound the exact per-entry-drop count this test is isolating. + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: raw }, maxIterations: 5 }); + expect(parsed.spec.networkAllowlist.ecosystems).toEqual([]); + // 8 warnings (one per processed, capped entry), not 100 -- proves the slice happened before the loop. + expect(parsed.warnings.length).toBe(8); + }); + + it("ecosystems: rejects a non-array value with a warning, falling back to defaults", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: "npm" } }); + expect(parsed.spec.networkAllowlist.ecosystems).toEqual([]); + expect(parsed.warnings.join(" ")).toMatch(/networkAllowlist\.ecosystems.*must be an array/i); + }); + + it("drops malformed extraHosts entries (invalid hostname shape, non-string, duplicate) with a warning each", () => { + const parsed = parseAmsPolicySpec({ + networkAllowlist: { extraHosts: ["good.example.com", "not a hostname", "-leading-hyphen.com", 7, "good.example.com"] }, + }); + expect(parsed.spec.networkAllowlist.extraHosts).toEqual(["good.example.com"]); + expect(parsed.warnings.length).toBeGreaterThanOrEqual(3); + }); + + it("caps extraHosts at 50 raw entries instead of processing an unbounded list", () => { + const raw = Array.from({ length: 200 }, (_, i) => `not valid host ${i}`); + // Same reasoning as the ecosystems cap test above -- isolates the per-entry-drop count. + const parsed = parseAmsPolicySpec({ networkAllowlist: { extraHosts: raw }, maxIterations: 5 }); + expect(parsed.spec.networkAllowlist.extraHosts).toEqual([]); + expect(parsed.warnings.length).toBe(50); + }); + + it("extraHosts: rejects a non-array value with a warning, falling back to defaults", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { extraHosts: { host: "example.com" } } }); + expect(parsed.spec.networkAllowlist.extraHosts).toEqual([]); + expect(parsed.warnings.join(" ")).toMatch(/networkAllowlist\.extraHosts.*must be an array/i); + }); + + it("resolves each field independently when only one is present", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm"] } }); + expect(parsed.spec.networkAllowlist).toEqual({ ecosystems: ["npm"], extraHosts: [] }); + }); + + it("rejects a non-mapping networkAllowlist value and falls back to defaults", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: ["npm"] }); + expect(parsed.spec.networkAllowlist).toEqual(DEFAULT_AMS_POLICY_SPEC.networkAllowlist); + expect(parsed.warnings.join(" ")).toMatch(/networkAllowlist.*must be a mapping/i); + }); + + it("null/undefined fall back silently, with no warning of their own", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: null, maxIterations: 5 }); + expect(parsed.spec.networkAllowlist).toEqual({ ecosystems: [], extraHosts: [] }); + expect(parsed.warnings).toEqual([]); + }); + + it("marks a non-default allowlist as configured even with no other field set", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm"] } }); + expect(parsed.present).toBe(true); + }); + + it("REGRESSION: an explicitly-empty allowlist does NOT count as configured", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: [], extraHosts: [] } }); + expect(parsed.present).toBe(false); + expect(parsed.spec).toEqual(DEFAULT_AMS_POLICY_SPEC); + }); + + it("mutating the returned spec's arrays never affects the shared DEFAULT_AMS_POLICY_SPEC singleton (empty input)", () => { + const parsed = parseAmsPolicySpec({}); + parsed.spec.networkAllowlist.ecosystems.push("npm"); + parsed.spec.networkAllowlist.extraHosts.push("evil.example.com"); + expect(DEFAULT_AMS_POLICY_SPEC.networkAllowlist).toEqual({ ecosystems: [], extraHosts: [] }); + }); + + it("REGRESSION: mutating the resolved arrays never affects the shared singleton even when networkAllowlist itself is untouched but a sibling field IS configured", () => { + // Distinct from the empty-input case above: hasConfiguredPolicyFields is true here (maxIterations + // differs from default), so this spec is built via the `present: true` return path, NOT + // cloneDefaultAmsPolicySpec() -- networkAllowlist's own normalizer has to defensively copy on its own, + // since nothing upstream of it does so on this path. + const parsed = parseAmsPolicySpec({ maxIterations: 5 }); + parsed.spec.networkAllowlist.ecosystems.push("npm"); + parsed.spec.networkAllowlist.extraHosts.push("evil.example.com"); + expect(DEFAULT_AMS_POLICY_SPEC.networkAllowlist).toEqual({ ecosystems: [], extraHosts: [] }); + }); + + it("REGRESSION: same protection when networkAllowlist is a partial object (one field specified, the other's default array must still be copied, not shared)", () => { + const parsed = parseAmsPolicySpec({ networkAllowlist: { ecosystems: ["npm"] } }); + parsed.spec.networkAllowlist.extraHosts.push("evil.example.com"); + expect(DEFAULT_AMS_POLICY_SPEC.networkAllowlist.extraHosts).toEqual([]); + }); + }); + it("maxIterations/maxTurnsPerIteration floor to whole counts, allow zero, and reject negative/non-numeric values", () => { const floored = parseAmsPolicySpec({ maxIterations: 4.9, maxTurnsPerIteration: 8.2 }); expect(floored.spec.maxIterations).toBe(4);