Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions packages/gittensory-miner/lib/deny-hook-synthesis.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,19 @@ export type DenyHookSynthesisStore = {
repoFullName: string,
history: unknown,
config?: SynthesisConfig,
apiBaseUrl?: string,
): DenyRuleProposal[];
listProposals(repoFullName: string): DenyRuleProposal[];
setProposalStatus(repoFullName: string, proposalId: string, status: DenyRuleProposalStatus): void;
resolveEffectiveRules(repoFullName: string, options?: { includeDefaults?: boolean }): DenyRule[];
listProposals(repoFullName: string, apiBaseUrl?: string): DenyRuleProposal[];
setProposalStatus(
repoFullName: string,
proposalId: string,
status: DenyRuleProposalStatus,
apiBaseUrl?: string,
): void;
resolveEffectiveRules(
repoFullName: string,
options?: { includeDefaults?: boolean; apiBaseUrl?: string },
): DenyRule[];
close(): void;
};

Expand Down
78 changes: 64 additions & 14 deletions packages/gittensory-miner/lib/deny-hook-synthesis.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { dirname, join } from "node:path";
import { createHash } from "node:crypto";
import { DatabaseSync } from "node:sqlite";
import { DEFAULT_DENY_RULES, evaluateDenyHooks } from "./deny-hooks.js";
import { DEFAULT_FORGE_CONFIG } from "./forge-config.js";

const defaultDbFileName = "deny-hook-synthesis.sqlite3";
const PROPOSAL_STATUSES = Object.freeze(["proposed", "approved", "rejected"]);
Expand All @@ -26,6 +27,14 @@ function normalizeRepoFullName(repoFullName) {
return `${owner}/${repo}`;
}

/** Optional forge host, scoping rows so two hosts serving the same owner/repo name never collide (#5563).
* Omitted/nullish → the github.com default, so every pre-existing single-forge caller is unaffected. */
function normalizeApiBaseUrl(apiBaseUrl) {
if (apiBaseUrl === undefined || apiBaseUrl === null) return DEFAULT_FORGE_CONFIG.apiBaseUrl;
if (typeof apiBaseUrl !== "string" || !apiBaseUrl.trim()) throw new Error("invalid_api_base_url");
return apiBaseUrl.trim();
}

function normalizeOptionalStringArray(value) {
if (value === undefined || value === null) return [];
if (!Array.isArray(value)) return [];
Expand Down Expand Up @@ -255,6 +264,42 @@ function rowToProposal(row) {
};
}

// Rebuild deny_rule_proposals' (repo_full_name, id) PRIMARY KEY into a (api_base_url, repo_full_name, id)
// composite (#5563) -- two forge hosts serving a same-named owner/repo must not share one proposal row. SQLite
// cannot ALTER a PRIMARY KEY in place, so this rebuilds the table: create the new shape, copy every existing row
// with the pre-#4784 implicit single-forge default backfilled, drop the old table, rename the new one in.
// Guarded by a column-presence check (this module has no schema-version framework of its own, unlike the
// package's other local stores) so this only runs once per file.
function ensureDenyRuleProposalsForgeScope(db) {
const hasApiBaseUrlColumn = db
.prepare("PRAGMA table_info(deny_rule_proposals)")
.all()
.some((column) => column.name === "api_base_url");
if (hasApiBaseUrlColumn) return;
db.exec(`
CREATE TABLE deny_rule_proposals_v2 (
api_base_url TEXT NOT NULL,
repo_full_name TEXT NOT NULL,
id TEXT NOT NULL,
status TEXT NOT NULL CHECK (status IN ('proposed', 'approved', 'rejected')),
rule_json TEXT NOT NULL,
audit_json TEXT NOT NULL,
updated_at TEXT NOT NULL,
PRIMARY KEY (api_base_url, repo_full_name, id)
)
`);
// OR IGNORE: a row this store's own read path already treats as unusable garbage (an unrecognized `status`,
// e.g. from a hand-edited or otherwise corrupted file) would violate the CHECK constraint above and abort the
// whole migration. Skipping it here is consistent with that same fail-closed posture, rather than turning one
// bad row into a permanently unmigratable file.
db.prepare(
`INSERT OR IGNORE INTO deny_rule_proposals_v2 (api_base_url, repo_full_name, id, status, rule_json, audit_json, updated_at)
SELECT ?, repo_full_name, id, status, rule_json, audit_json, updated_at FROM deny_rule_proposals`,
).run(DEFAULT_FORGE_CONFIG.apiBaseUrl);
db.exec("DROP TABLE deny_rule_proposals");
db.exec("ALTER TABLE deny_rule_proposals_v2 RENAME TO deny_rule_proposals");
}

/**
* Local SQLite store for synthesized deny-rule proposals. Refresh re-derives proposals from history while
* preserving maintainer decisions on ids that still exist.
Expand All @@ -276,40 +321,43 @@ export function initDenyHookSynthesisStore(dbPath = resolveDenyHookSynthesisDbPa
PRIMARY KEY (repo_full_name, id)
)
`);
ensureDenyRuleProposalsForgeScope(db);

const upsertStatement = db.prepare(`
INSERT INTO deny_rule_proposals (repo_full_name, id, status, rule_json, audit_json, updated_at)
VALUES (?, ?, ?, ?, ?, ?)
ON CONFLICT(repo_full_name, id) DO UPDATE SET
INSERT INTO deny_rule_proposals (api_base_url, repo_full_name, id, status, rule_json, audit_json, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?)
ON CONFLICT(api_base_url, repo_full_name, id) DO UPDATE SET
status = excluded.status,
rule_json = excluded.rule_json,
audit_json = excluded.audit_json,
updated_at = excluded.updated_at
`);
const getStatusStatement = db.prepare(
"SELECT status FROM deny_rule_proposals WHERE repo_full_name = ? AND id = ?",
"SELECT status FROM deny_rule_proposals WHERE api_base_url = ? AND repo_full_name = ? AND id = ?",
);
const listStatement = db.prepare(
"SELECT repo_full_name, id, status, rule_json, audit_json, updated_at FROM deny_rule_proposals WHERE repo_full_name = ? ORDER BY id ASC",
"SELECT repo_full_name, id, status, rule_json, audit_json, updated_at FROM deny_rule_proposals WHERE api_base_url = ? AND repo_full_name = ? ORDER BY id ASC",
);
const setStatusStatement = db.prepare(`
UPDATE deny_rule_proposals SET status = ?, updated_at = ? WHERE repo_full_name = ? AND id = ?
UPDATE deny_rule_proposals SET status = ?, updated_at = ? WHERE api_base_url = ? AND repo_full_name = ? AND id = ?
`);

return {
dbPath: resolvedPath,
refreshProposals(repoFullName, history, config = {}) {
refreshProposals(repoFullName, history, config = {}, apiBaseUrl) {
const forge = normalizeApiBaseUrl(apiBaseUrl);
const repo = normalizeRepoFullName(repoFullName);
const synthesized = synthesizeDenyRuleProposals(history, config);
const updatedAt = new Date().toISOString();
db.exec("BEGIN IMMEDIATE");
try {
for (const proposal of synthesized) {
const existing = getStatusStatement.get(repo, proposal.id);
const existing = getStatusStatement.get(forge, repo, proposal.id);
const status = existing?.status && proposalStatusSet.has(existing.status) && existing.status !== "proposed"
? existing.status
: "proposed";
upsertStatement.run(
forge,
repo,
proposal.id,
status,
Expand All @@ -323,20 +371,22 @@ export function initDenyHookSynthesisStore(dbPath = resolveDenyHookSynthesisDbPa
db.exec("ROLLBACK");
throw error;
}
return listStatement.all(repo).map(rowToProposal);
return listStatement.all(forge, repo).map(rowToProposal);
},
listProposals(repoFullName) {
listProposals(repoFullName, apiBaseUrl) {
const forge = normalizeApiBaseUrl(apiBaseUrl);
const repo = normalizeRepoFullName(repoFullName);
return listStatement.all(repo).map(rowToProposal);
return listStatement.all(forge, repo).map(rowToProposal);
},
setProposalStatus(repoFullName, proposalId, status) {
setProposalStatus(repoFullName, proposalId, status, apiBaseUrl) {
const forge = normalizeApiBaseUrl(apiBaseUrl);
const repo = normalizeRepoFullName(repoFullName);
if (typeof proposalId !== "string" || !proposalId.trim()) throw new Error("invalid_proposal_id");
if (!proposalStatusSet.has(status)) throw new Error("invalid_proposal_status");
setStatusStatement.run(status, new Date().toISOString(), repo, proposalId.trim());
setStatusStatement.run(status, new Date().toISOString(), forge, repo, proposalId.trim());
},
resolveEffectiveRules(repoFullName, options = {}) {
const proposals = this.listProposals(repoFullName);
const proposals = this.listProposals(repoFullName, options.apiBaseUrl);
return resolveEffectiveDenyRules({
includeDefaults: options.includeDefaults,
approvedProposals: proposals,
Expand Down
141 changes: 141 additions & 0 deletions test/unit/miner-deny-hook-synthesis.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { DatabaseSync } from "node:sqlite";
import { afterEach, describe, expect, it } from "vitest";
import {
DEFAULT_DENY_RULES,
Expand Down Expand Up @@ -112,4 +113,144 @@ describe("initDenyHookSynthesisStore() (#4522)", () => {
const effective = store.resolveEffectiveRules("acme/widgets");
expect(effective.length).toBe(DEFAULT_DENY_RULES.length + 1);
});

describe("forge-scoping (#5563)", () => {
it("two forge hosts can each hold their own proposals for the same owner/repo without colliding", () => {
const store = tempStore();
const history = [
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
];
const ghRefreshed = store.refreshProposals("acme/widgets", history, {}, "https://github.com/ghapi");
const gheRefreshed = store.refreshProposals("acme/widgets", history, {}, "https://ghe.example.com/api/v3");
expect(ghRefreshed).toHaveLength(1);
expect(gheRefreshed).toHaveLength(1);
// Same synthesized proposal id (derived from the path, not the host) on both hosts, but the rows are
// independent -- approving one host's proposal must not affect the other's.
expect(ghRefreshed[0]!.id).toBe(gheRefreshed[0]!.id);
store.setProposalStatus("acme/widgets", ghRefreshed[0]!.id, "approved", "https://github.com/ghapi");
expect(store.listProposals("acme/widgets", "https://github.com/ghapi")[0]?.status).toBe("approved");
expect(store.listProposals("acme/widgets", "https://ghe.example.com/api/v3")[0]?.status).toBe("proposed");
});

it("defaults apiBaseUrl to the github.com default when omitted", () => {
const store = tempStore();
const history = [
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
];
store.refreshProposals("acme/widgets", history);
expect(store.listProposals("acme/widgets", "https://github.com/ghapi")).toHaveLength(1);
});

it("resolveEffectiveRules threads options.apiBaseUrl through to listProposals", () => {
const store = tempStore();
const history = [
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
{ blockerCodes: ["guardrail_hold"], changedPaths: ["CHANGELOG.md"] },
];
const refreshed = store.refreshProposals("acme/widgets", history, {}, "https://ghe.example.com/api/v3");
store.setProposalStatus("acme/widgets", refreshed[0]!.id, "approved", "https://ghe.example.com/api/v3");

// The github.com host has no approved proposal -- effective rules stay at the static defaults.
expect(store.resolveEffectiveRules("acme/widgets").length).toBe(DEFAULT_DENY_RULES.length);
// The GHE host's approval is picked up when its apiBaseUrl is threaded through.
expect(
store.resolveEffectiveRules("acme/widgets", { apiBaseUrl: "https://ghe.example.com/api/v3" }).length,
).toBe(DEFAULT_DENY_RULES.length + 1);
});

it("rejects a non-string or blank apiBaseUrl", () => {
const store = tempStore();
expect(() => store.listProposals("acme/widgets", " ")).toThrow("invalid_api_base_url");
expect(() => store.refreshProposals("acme/widgets", [], {}, 42 as never)).toThrow("invalid_api_base_url");
expect(() => store.setProposalStatus("acme/widgets", "x", "approved", " ")).toThrow("invalid_api_base_url");
});

it("migrates an existing pre-#5563 file, backfilling api_base_url and preserving every row", () => {
const dir = mkdtempSync(join(tmpdir(), "miner-deny-hook-synthesis-legacy-"));
tempDirs.push(dir);
const dbPath = join(dir, "legacy.sqlite3");
const legacy = new DatabaseSync(dbPath);
legacy.exec(`
CREATE TABLE deny_rule_proposals (
repo_full_name TEXT NOT NULL,
id TEXT NOT NULL,
status TEXT NOT NULL CHECK (status IN ('proposed', 'approved', 'rejected')),
rule_json TEXT NOT NULL,
audit_json TEXT NOT NULL,
updated_at TEXT NOT NULL,
PRIMARY KEY (repo_full_name, id)
)
`);
legacy
.prepare(
"INSERT INTO deny_rule_proposals (repo_full_name, id, status, rule_json, audit_json, updated_at) VALUES (?, ?, ?, ?, ?, ?)",
)
.run(
"acme/widgets",
"path:abc123",
"approved",
JSON.stringify({ matcher: "*", pathPattern: "**/changelog.md", reason: "legacy" }),
JSON.stringify({ kind: "path_history", synthesizedAt: "2026-01-01T00:00:00.000Z" }),
"2026-01-01T00:00:00.000Z",
);
legacy.close();

const store = initDenyHookSynthesisStore(dbPath);
stores.push(store);
expect(store.listProposals("acme/widgets", "https://github.com/ghapi")).toEqual([
{
id: "path:abc123",
status: "approved",
rule: { matcher: "*", pathPattern: "**/changelog.md", reason: "legacy" },
audit: { kind: "path_history", synthesizedAt: "2026-01-01T00:00:00.000Z" },
},
]);
// The old bare (repo_full_name, id) collision is gone: a second host can now hold its own proposal state.
store.setProposalStatus("acme/widgets", "path:abc123", "rejected", "https://ghe.example.com/api/v3");
expect(store.listProposals("acme/widgets", "https://github.com/ghapi")[0]?.status).toBe("approved");
});

it("REGRESSION: a legacy row violating the rebuilt table's status CHECK constraint is dropped, not a migration-aborting crash", () => {
const dir = mkdtempSync(join(tmpdir(), "miner-deny-hook-synthesis-legacy-corrupt-"));
tempDirs.push(dir);
const dbPath = join(dir, "legacy-corrupt.sqlite3");
const legacy = new DatabaseSync(dbPath);
// No CHECK on status here, simulating a hand-edited or otherwise corrupted legacy file -- the real
// baseline schema always enforces the CHECK, so this can only arise from external tampering.
legacy.exec(`
CREATE TABLE deny_rule_proposals (
repo_full_name TEXT NOT NULL,
id TEXT NOT NULL,
status TEXT NOT NULL,
rule_json TEXT NOT NULL,
audit_json TEXT NOT NULL,
updated_at TEXT NOT NULL,
PRIMARY KEY (repo_full_name, id)
)
`);
legacy
.prepare(
"INSERT INTO deny_rule_proposals (repo_full_name, id, status, rule_json, audit_json, updated_at) VALUES (?, ?, ?, ?, ?, ?)",
)
.run("acme/corrupt", "path:bad", "bogus", "{}", "{}", "2026-01-01T00:00:00.000Z");
legacy
.prepare(
"INSERT INTO deny_rule_proposals (repo_full_name, id, status, rule_json, audit_json, updated_at) VALUES (?, ?, ?, ?, ?, ?)",
)
.run("acme/widgets", "path:ok", "proposed", "{}", "{}", "2026-01-01T00:00:00.000Z");
legacy.close();

let opened: ReturnType<typeof initDenyHookSynthesisStore> | undefined;
expect(() => {
opened = initDenyHookSynthesisStore(dbPath);
}).not.toThrow();
const store = opened!;
stores.push(store);
// The corrupt row was dropped, not migrated -- only the valid row survived the rebuild.
expect(store.listProposals("acme/corrupt", "https://github.com/ghapi")).toEqual([]);
expect(store.listProposals("acme/widgets", "https://github.com/ghapi")).toHaveLength(1);
});
});
});