Skip to content

Commit 9f7aeaf

Browse files
devcxldevcxl
authored
feat(tdd): D4b — 独立 GitHub broker + 最小凭证隔离
- 新增 BrokerCredentials 类型，FlowBroker 构造时接收可选独立 token - #credentials 使用 ES private field，JSON.stringify/Object.keys 无法访问 - verifyCredentials() 通过 gh auth status 校验凭证有效性 - writeFlowRunWithLock 将 broker 凭证注入 read/write gh 操作 - gh() 新增可选 env 参数，支持 GH_TOKEN 覆盖 - readFlowRunWithLock/writeFlowRunWithLock 新增可选 ghEnv 参数 - 测试：凭证隐私验证、verifyCredentials、凭证传递、shell 隔离检查
1 parent 7b46322 commit 9f7aeaf

5 files changed

Lines changed: 269 additions & 20 deletions

File tree

‎src/flowrun/github.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,33 +65,33 @@ export function replaceFlowRunInBody(body: string, flowRun: FlowRun): string {
6565
return body.slice(0, startIdx) + block + body.slice(endIdx + CABINET_END_MARKER.length)
6666
}
6767

68-
export async function readFlowRun(issueNumber: number): Promise<FlowRunReadResult> {
68+
export async function readFlowRun(issueNumber: number, ghEnv?: Record<string, string>): Promise<FlowRunReadResult> {
6969
try {
70-
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`)
70+
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`, 30_000, ghEnv)
7171
return extractFlowRunFromBody(stdout)
7272
} catch {
7373
return { ok: false, code: "NOT_FOUND", errors: [{ path: "", message: "Failed to read issue body" }] }
7474
}
7575
}
7676

77-
export async function writeFlowRun(issueNumber: number, flowRun: FlowRun): Promise<{ success: boolean; error?: string }> {
77+
export async function writeFlowRun(issueNumber: number, flowRun: FlowRun, ghEnv?: Record<string, string>): Promise<{ success: boolean; error?: string }> {
7878
try {
79-
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`)
79+
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`, 30_000, ghEnv)
8080
const body = stdout
8181
const newBody = replaceFlowRunInBody(body, flowRun)
8282

8383
const escaped = escapeShellArg(newBody)
8484

85-
await gh(`issue edit ${issueNumber} --body '${escaped}'`)
85+
await gh(`issue edit ${issueNumber} --body '${escaped}'`, 30_000, ghEnv)
8686
return { success: true }
8787
} catch (err) {
8888
return { success: false, error: String(err) }
8989
}
9090
}
9191

92-
export async function readFlowRunWithLock(issueNumber: number): Promise<{ flowRunResult: FlowRunReadResult; currentBody: string | null }> {
92+
export async function readFlowRunWithLock(issueNumber: number, ghEnv?: Record<string, string>): Promise<{ flowRunResult: FlowRunReadResult; currentBody: string | null }> {
9393
try {
94-
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`)
94+
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`, 30_000, ghEnv)
9595
const flowRunResult = extractFlowRunFromBody(stdout)
9696
return { flowRunResult, currentBody: stdout }
9797
} catch {
@@ -103,35 +103,36 @@ export async function writeFlowRunWithLock(
103103
issueNumber: number,
104104
flowRun: FlowRun,
105105
previousBody: string,
106+
ghEnv?: Record<string, string>,
106107
): Promise<{ success: boolean; error?: string; conflict: boolean }> {
107108
try {
108-
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`)
109+
const { stdout } = await gh(`issue view ${issueNumber} --json body --jq .body`, 30_000, ghEnv)
109110

110111
if (previousBody !== stdout) {
111112
return { success: false, error: "Conflict: body has changed since last read", conflict: true }
112113
}
113114

114115
const newBody = replaceFlowRunInBody(stdout, flowRun)
115116
const escaped = escapeShellArg(newBody)
116-
await gh(`issue edit ${issueNumber} --body '${escaped}'`)
117+
await gh(`issue edit ${issueNumber} --body '${escaped}'`, 30_000, ghEnv)
117118
return { success: true, conflict: false }
118119
} catch (err) {
119120
return { success: false, error: String(err), conflict: false }
120121
}
121122
}
122123

123-
export async function applyLabel(issueNumber: number, label: string): Promise<boolean> {
124+
export async function applyLabel(issueNumber: number, label: string, ghEnv?: Record<string, string>): Promise<boolean> {
124125
try {
125-
await gh(`issue edit ${issueNumber} --add-label '${label}'`)
126+
await gh(`issue edit ${issueNumber} --add-label '${label}'`, 30_000, ghEnv)
126127
return true
127128
} catch {
128129
return false
129130
}
130131
}
131132

132-
export async function removeLabel(issueNumber: number, label: string): Promise<boolean> {
133+
export async function removeLabel(issueNumber: number, label: string, ghEnv?: Record<string, string>): Promise<boolean> {
133134
try {
134-
await gh(`issue edit ${issueNumber} --remove-label '${label}'`)
135+
await gh(`issue edit ${issueNumber} --remove-label '${label}'`, 30_000, ghEnv)
135136
return true
136137
} catch {
137138
return false

‎src/plugin/broker.ts‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ import { readFlowRunWithLock, writeFlowRunWithLock as ghWriteFlowRunWithLock } f
33

44
// ─── 类型 ───
55

6+
export interface BrokerCredentials {
7+
/** 独立 GitHub API token（不传入 Agent shell） */
8+
token: string
9+
}
10+
611
export interface MutateResult<R> {
712
flowRun: FlowRun
813
result: R
@@ -48,11 +53,56 @@ class KeyedMutex {
4853
* 1. 按 parentIssueNumber 的 keyed mutex 串行所有 FlowRun 写入
4954
* 2. read-modify-write 循环（乐观锁）
5055
* 3. 检测外部 body 变化 → PERSIST_CONFLICT
56+
* 4. 持有独立 GitHub API 凭证，Agent shell 不可访问
5157
*
5258
* 所有 FlowRun/Task/Evidence 写入必须经过 broker，不直接暴露 GitHub API。
59+
*
60+
* 凭证安全：credentials 存储在闭包变量中，不挂载在 this 上，
61+
* JSON.stringify / Object.keys 无法访问。
5362
*/
5463
export class FlowBroker {
5564
private mutex = new KeyedMutex()
65+
#credentials: BrokerCredentials | null
66+
67+
/**
68+
* @param credentials 可选。独立 GitHub API 凭证。
69+
* 传入时：所有 gh 操作使用该 token，而非 ambient 环境。
70+
* 不传时：降级使用 ambient 环境变量（GH_TOKEN etc.）。
71+
*/
72+
constructor(credentials?: BrokerCredentials) {
73+
this.#credentials = credentials ?? null
74+
}
75+
76+
/**
77+
* 获取凭证对应的环境变量覆盖（仅内部使用）。
78+
* 返回 undefined 表示无独立凭证，应使用 ambient 环境。
79+
*/
80+
private get ghEnv(): Record<string, string> | undefined {
81+
if (!this.#credentials) return undefined
82+
return {
83+
GH_TOKEN: this.#credentials.token,
84+
GITHUB_TOKEN: this.#credentials.token,
85+
}
86+
}
87+
88+
/**
89+
* 验证 broker 持有的 GitHub 凭证是否有效。
90+
*
91+
* 调用 `gh auth status` 检查 token 可用性。
92+
* 无凭证时返回 ok: false。
93+
*/
94+
async verifyCredentials(): Promise<{ ok: boolean; message: string }> {
95+
if (!this.#credentials) {
96+
return { ok: false, message: "No broker credentials configured" }
97+
}
98+
try {
99+
const { gh } = await import("../util/gh.js")
100+
await gh("auth status", 15_000, this.ghEnv)
101+
return { ok: true, message: "Broker credentials are valid" }
102+
} catch (err) {
103+
return { ok: false, message: `Broker credentials verification failed: ${String(err)}` }
104+
}
105+
}
56106

57107
/**
58108
* 将操作加入 keyed mutex 队列。
@@ -70,14 +120,15 @@ export class FlowBroker {
70120
* - shouldPersist=false 时跳过写入（如校验失败）
71121
* - 自动增加 revision 和时间戳
72122
* - body 不匹配时返回 PERSIST_CONFLICT
123+
* - 使用 broker 独立凭证执行 gh 操作
73124
*/
74125
async writeFlowRunWithLock<R>(
75126
issueNumber: number,
76127
handler: (flowRun: FlowRun) => MutateResult<R>,
77128
): Promise<WriteResult<R>> {
78129
return this.mutex.runExclusive(issueNumber, async () => {
79-
// ── 读取 ──
80-
const { flowRunResult, currentBody } = await readFlowRunWithLock(issueNumber)
130+
// ── 读取（broker 凭证） ──
131+
const { flowRunResult, currentBody } = await readFlowRunWithLock(issueNumber, this.ghEnv)
81132
if (!flowRunResult.ok || currentBody === null) {
82133
return {
83134
ok: false as const,
@@ -98,11 +149,11 @@ export class FlowBroker {
98149
}
99150
}
100151

101-
// ── 写入（乐观锁） ──
152+
// ── 写入（乐观锁，broker 凭证） ──
102153
updated.revision += 1
103154
updated.lastTickAt = new Date().toISOString()
104155

105-
const writeResult = await ghWriteFlowRunWithLock(issueNumber, updated, currentBody)
156+
const writeResult = await ghWriteFlowRunWithLock(issueNumber, updated, currentBody, this.ghEnv)
106157
if (!writeResult.success) {
107158
return {
108159
ok: false as const,

‎src/util/gh.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@ import { promisify } from "node:util"
33

44
const execAsync = promisify(exec)
55

6-
export function gh(args: string, timeout = 30_000): Promise<{ stdout: string; stderr: string }> {
7-
return execAsync(`gh ${args}`, { timeout })
6+
/**
7+
* 执行 gh CLI 命令。
8+
*
9+
* @param args gh 子命令及参数
10+
* @param timeout 超时毫秒数（默认 30s）
11+
* @param env 可选的环境变量覆盖（如注入 GH_TOKEN）
12+
*/
13+
export function gh(args: string, timeout = 30_000, env?: Record<string, string>): Promise<{ stdout: string; stderr: string }> {
14+
const execEnv = env ? { ...process.env, ...env } : process.env
15+
return execAsync(`gh ${args}`, { timeout, env: execEnv })
816
}

‎test/plugin/broker.test.ts‎

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ vi.mock("../../src/flowrun/github.js", async () => {
1919
}
2020
})
2121

22+
// ─── Mock gh util (for verifyCredentials tests) ───
23+
24+
const mockGhFn = vi.fn()
25+
26+
vi.mock("../../src/util/gh.js", () => ({
27+
gh: (...args: unknown[]) => mockGhFn(...args),
28+
}))
29+
2230
// ─── 辅助函数 ───
2331

2432
function delay(ms: number): Promise<void> {
@@ -101,6 +109,136 @@ beforeEach(() => {
101109
vi.clearAllMocks()
102110
})
103111

112+
// ─── 独立凭证测试 ───
113+
114+
describe("FlowBroker — 独立凭证", () => {
115+
it("接受 BrokerCredentials 构造参数", () => {
116+
const broker = new FlowBroker({ token: "ghp_test123" })
117+
expect(broker).toBeDefined()
118+
})
119+
120+
it("无凭证构造时正常运行（降级 ambient）", () => {
121+
const broker = new FlowBroker()
122+
expect(broker).toBeDefined()
123+
})
124+
125+
it("broker token 不在 JSON 序列化输出中", () => {
126+
const broker = new FlowBroker({ token: "ghp_secret_do_not_leak" })
127+
const serialized = JSON.stringify(broker)
128+
expect(serialized).not.toContain("ghp_secret_do_not_leak")
129+
})
130+
131+
it("broker token 不通过 Object.keys 暴露", () => {
132+
const broker = new FlowBroker({ token: "ghp_secret" })
133+
const valueStr = JSON.stringify(Object.values(broker as unknown as Record<string, unknown>))
134+
// Token 不应作为可枚举属性出现
135+
expect(valueStr).not.toContain("ghp_secret")
136+
})
137+
})
138+
139+
describe("FlowBroker — verifyCredentials", () => {
140+
it("无凭证时返回 ok: false", async () => {
141+
const broker = new FlowBroker()
142+
const result = await broker.verifyCredentials()
143+
expect(result.ok).toBe(false)
144+
expect(result.message).toContain("No broker credentials")
145+
// 无凭证时不应调用 gh
146+
expect(mockGhFn).not.toHaveBeenCalled()
147+
})
148+
149+
it("凭证有效时返回 ok: true", async () => {
150+
mockGhFn.mockResolvedValue({ stdout: "", stderr: "" })
151+
152+
const broker = new FlowBroker({ token: "ghp_valid_token" })
153+
const result = await broker.verifyCredentials()
154+
155+
expect(result.ok).toBe(true)
156+
expect(mockGhFn).toHaveBeenCalledWith(
157+
"auth status",
158+
15_000,
159+
expect.objectContaining({ GH_TOKEN: "ghp_valid_token", GITHUB_TOKEN: "ghp_valid_token" }),
160+
)
161+
})
162+
163+
it("凭证无效时返回 ok: false", async () => {
164+
mockGhFn.mockRejectedValue(new Error("Authentication failed"))
165+
166+
const broker = new FlowBroker({ token: "ghp_invalid" })
167+
const result = await broker.verifyCredentials()
168+
169+
expect(result.ok).toBe(false)
170+
expect(result.message).toContain("failed")
171+
})
172+
})
173+
174+
describe("FlowBroker — 凭证传递给 GitHub 操作", () => {
175+
it("有凭证时 readFlowRunWithLock 收到 ghEnv", async () => {
176+
const broker = new FlowBroker({ token: "ghp_broker" })
177+
const flowRun = makeFlowRun()
178+
setupReadWrite(flowRun)
179+
180+
await broker.writeFlowRunWithLock(1, (fr) => {
181+
fr.status = "completed"
182+
return { flowRun: fr, result: 42, shouldPersist: true }
183+
})
184+
185+
expect(mockReadFlowRunWithLock).toHaveBeenCalledWith(
186+
1,
187+
expect.objectContaining({ GH_TOKEN: "ghp_broker", GITHUB_TOKEN: "ghp_broker" }),
188+
)
189+
})
190+
191+
it("有凭证时 writeFlowRunWithLock 收到 ghEnv", async () => {
192+
const broker = new FlowBroker({ token: "ghp_broker" })
193+
const flowRun = makeFlowRun()
194+
setupReadWrite(flowRun)
195+
196+
await broker.writeFlowRunWithLock(1, (fr) => {
197+
fr.status = "completed"
198+
return { flowRun: fr, result: 42, shouldPersist: true }
199+
})
200+
201+
expect(mockWriteFlowRunWithLock).toHaveBeenCalledWith(
202+
1,
203+
expect.any(Object), // flowRun
204+
expect.any(String), // currentBody
205+
expect.objectContaining({ GH_TOKEN: "ghp_broker", GITHUB_TOKEN: "ghp_broker" }),
206+
)
207+
})
208+
209+
it("无凭证时不传递 ghEnv（undefined）", async () => {
210+
const broker = new FlowBroker() // no credentials
211+
const flowRun = makeFlowRun()
212+
setupReadWrite(flowRun)
213+
214+
await broker.writeFlowRunWithLock(1, (fr) => {
215+
fr.status = "completed"
216+
return { flowRun: fr, result: 42, shouldPersist: true }
217+
})
218+
219+
expect(mockReadFlowRunWithLock).toHaveBeenCalledWith(1, undefined)
220+
expect(mockWriteFlowRunWithLock).toHaveBeenCalledWith(
221+
1,
222+
expect.any(Object),
223+
expect.any(String),
224+
undefined,
225+
)
226+
})
227+
228+
it("enqueue 操作不传递 ghEnv（enqueue 不调用 gh）", async () => {
229+
const broker = new FlowBroker({ token: "ghp_broker" })
230+
let result = ""
231+
await broker.enqueue(1, async () => {
232+
result = "done"
233+
return result
234+
})
235+
expect(result).toBe("done")
236+
// enqueue 不应调用任何 gh 操作
237+
expect(mockReadFlowRunWithLock).not.toHaveBeenCalled()
238+
expect(mockWriteFlowRunWithLock).not.toHaveBeenCalled()
239+
})
240+
})
241+
104242
// ─── Keyed Mutex 测试 ───
105243

106244
describe("FlowBroker — enqueue (keyed mutex)", () => {
@@ -210,7 +348,7 @@ describe("FlowBroker — writeFlowRunWithLock", () => {
210348
}
211349

212350
// 验证 readFlowRunWithLock 被调用
213-
expect(mockReadFlowRunWithLock).toHaveBeenCalledWith(1)
351+
expect(mockReadFlowRunWithLock).toHaveBeenCalledWith(1, undefined)
214352
// 验证 writeFlowRunWithLock 被调用，且 revision 已 bump
215353
expect(mockWriteFlowRunWithLock).toHaveBeenCalledTimes(1)
216354
const writtenFlowRun = mockWriteFlowRunWithLock.mock.calls[0][1] as FlowRun

0 commit comments

Comments
 (0)