From 38fd2ede0a34f644e7937901b624dc55262f772e Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 28 Sep 2026 12:51:58 -0500 Subject: [PATCH] Close Copilot probe resources on failure and retain redaction context Co-Authored-By: Paperclip --- .../scripts/probe-copilot-acp.py | 155 ++++++++++-------- .../scripts/probe-copilot-acp.test.mjs | 9 + .../scripts/probe-copilot-acp.test.py | 93 +++++++++++ .../acpx/copilot-extension-adapter.test.ts | 23 ++- .../drivers/acpx/copilot-extension-adapter.ts | 9 +- 5 files changed, 218 insertions(+), 71 deletions(-) create mode 100644 packages/paperclip-runner/scripts/probe-copilot-acp.test.mjs create mode 100644 packages/paperclip-runner/scripts/probe-copilot-acp.test.py diff --git a/packages/paperclip-runner/scripts/probe-copilot-acp.py b/packages/paperclip-runner/scripts/probe-copilot-acp.py index 65f88b7d4ba..488f9a52e2e 100644 --- a/packages/paperclip-runner/scripts/probe-copilot-acp.py +++ b/packages/paperclip-runner/scripts/probe-copilot-acp.py @@ -2,7 +2,7 @@ No credentials are inherited; COPILOT_OFFLINE disables provider network access. This proves a protocol fixture, never GitHub/model/Daytona qualification. """ -import argparse,json,subprocess,tempfile,pathlib,os,select,time,threading,http.server,hashlib,shutil +import argparse,json,subprocess,tempfile,pathlib,os,select,time,threading,http.server,hashlib,shutil,contextlib,sys parser=argparse.ArgumentParser(description=__doc__) parser.add_argument('--package-root',required=True) parser.add_argument('--scenario',choices=['deny-write','attached-shell'],default='deny-write') @@ -17,75 +17,96 @@ model_tool_names=[] command='sleep 2; printf ACP_SHELL_DONE > settlement.txt' binary=str(package_root/'copilot') -root=pathlib.Path(tempfile.mkdtemp(prefix='paperclip-copilot-offline-')) -calls=0 -class Handler(http.server.BaseHTTPRequestHandler): - def log_message(self,*args):pass - def do_GET(self): - self.send_response(200);self.send_header('Content-Type','application/json');self.end_headers();self.wfile.write(json.dumps({'data':[{'id':'gpt-4.1','object':'model','owned_by':'fixture'}]}).encode()) - def do_POST(self): - global calls,model_tool_names - calls+=1 - data=json.loads(self.rfile.read(int(self.headers.get('Content-Length','0')))) - model_tool_names=[x.get('function',{}).get('name') for x in data.get('tools',[])] - if calls>4:raise RuntimeError('Fixture model exceeded its bounded request count') - if data.get('stream'): - self.send_response(200);self.send_header('Content-Type','text/event-stream');self.end_headers() - values=[{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{'role':'assistant','content':'Fixture complete.'},'finish_reason':None}]},{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{},'finish_reason':'stop'}],'usage':{'prompt_tokens':10,'completion_tokens':3,'total_tokens':13}}] - if calls==1:values=[{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{'role':'assistant','tool_calls':[{'index':0,'id':'fixture-tool','type':'function','function':{'name':'create' if args.scenario=='deny-write' else 'bash','arguments':json.dumps({'path':str(root/'denied.txt'),'file_text':'MUST NOT EXIST'} if args.scenario=='deny-write' else {'command':command,'description':'Bounded settlement fixture','mode':'async','detach':False})}}]},'finish_reason':None}]},{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{},'finish_reason':'tool_calls'}]}] - for value in values:self.wfile.write(('data: '+json.dumps(value)+'\n\n').encode()) - self.wfile.write(b'data: [DONE]\n\n') - else: - self.send_response(200);self.send_header('Content-Type','application/json');self.end_headers();self.wfile.write(json.dumps({'id':'fixture-1','object':'chat.completion','choices':[{'index':0,'message':{'role':'assistant','content':'Fixture complete.'},'finish_reason':'stop'}],'usage':{'prompt_tokens':10,'completion_tokens':3,'total_tokens':13}}).encode()) -server=http.server.HTTPServer(('127.0.0.1',0),Handler);threading.Thread(target=server.serve_forever,daemon=True).start() -env={'PATH':'/usr/bin:/bin','HOME':str(root/'home'),'XDG_CONFIG_HOME':str(root/'config'),'XDG_CACHE_HOME':str(root/'cache'),'XDG_DATA_HOME':str(root/'data'),'COPILOT_HOME':str(root/'copilot'),'COPILOT_CACHE_HOME':str(root/'copilot-cache'),'COPILOT_AUTO_UPDATE':'false','COPILOT_OFFLINE':'true','COPILOT_PROVIDER_BASE_URL':f'http://127.0.0.1:{server.server_port}','COPILOT_PROVIDER_TYPE':'openai','COPILOT_PROVIDER_MODEL_ID':'gpt-4.1','COPILOT_MODEL':'gpt-4.1','NO_COLOR':'1'} -for key in ('HOME','XDG_CONFIG_HOME','XDG_CACHE_HOME','XDG_DATA_HOME','COPILOT_HOME','COPILOT_CACHE_HOME'):pathlib.Path(env[key]).mkdir() -(root/'copilot/config.json').write_text(json.dumps({'trustedFolders':[],'disableAllHooks':True,'memory':False,'ide':{'autoConnect':False}})) -p=subprocess.Popen([binary,'--acp','--stdio','--no-auto-update','--disable-builtin-mcps','--no-remote','--no-remote-export','--no-bash-env'],env=env,cwd=root,stdin=subprocess.PIPE,stdout=subprocess.PIPE,stderr=subprocess.PIPE) -responses={};wire=[];permission_responses=[];buf={p.stdout:b'',p.stderr:b''};nextid=0;marker_at_prompt_result=False +def stop_process(process): + # Cleanup must still reap a child whose stdin broke or which ignores TERM. + try: + if process.stdin:process.stdin.close() + except OSError:pass + try: + process.terminate() + try:process.wait(timeout=5) + except subprocess.TimeoutExpired:process.kill();process.wait(timeout=5) + finally: + for stream in (process.stdout,process.stderr): + if stream:stream.close() -def pump(wait=1): - for stream in select.select([p.stdout,p.stderr],[],[],wait)[0]: - data=os.read(stream.fileno(),65536) - if not data:continue - buf[stream]+=data - while b'\n' in buf[stream]: - line,buf[stream]=buf[stream].split(b'\n',1) - if not line:continue - if stream==p.stderr:continue - message=json.loads(line);wire.append(message) - if 'id' in message and 'method' not in message:responses[message['id']]=message - if message.get('method')=='session/request_permission': - allow=args.scenario=='attached-shell' and message['params'].get('toolCall',{}).get('rawInput',{}).get('command')==command - reject=next((o for o in message['params']['options'] if o['kind']==('allow_once' if allow else 'reject_once')),None) - outcome={'outcome':'selected','optionId':reject['optionId']} if reject else {'outcome':'cancelled'} - permission_responses.append({'id':message['id'],'outcome':outcome}) - p.stdin.write((json.dumps({'jsonrpc':'2.0','id':message['id'],'result':{'outcome':outcome}})+'\n').encode());p.stdin.flush() +with contextlib.ExitStack() as cleanup: + root=pathlib.Path(tempfile.mkdtemp(prefix='paperclip-copilot-offline-')) + cleanup.callback(shutil.rmtree, root) + calls=0 + session=None + marker_at_prompt_result=False + class Handler(http.server.BaseHTTPRequestHandler): + def log_message(self,*args):pass + def do_GET(self): + self.send_response(200);self.send_header('Content-Type','application/json');self.end_headers();self.wfile.write(json.dumps({'data':[{'id':'gpt-4.1','object':'model','owned_by':'fixture'}]}).encode()) + def do_POST(self): + global calls,model_tool_names + calls+=1 + data=json.loads(self.rfile.read(int(self.headers.get('Content-Length','0')))) + model_tool_names=[x.get('function',{}).get('name') for x in data.get('tools',[])] + if calls>4:raise RuntimeError('Fixture model exceeded its bounded request count') + if data.get('stream'): + self.send_response(200);self.send_header('Content-Type','text/event-stream');self.end_headers() + values=[{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{'role':'assistant','content':'Fixture complete.'},'finish_reason':None}]},{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{},'finish_reason':'stop'}],'usage':{'prompt_tokens':10,'completion_tokens':3,'total_tokens':13}}] + if calls==1:values=[{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{'role':'assistant','tool_calls':[{'index':0,'id':'fixture-tool','type':'function','function':{'name':'create' if args.scenario=='deny-write' else 'bash','arguments':json.dumps({'path':str(root/'denied.txt'),'file_text':'MUST NOT EXIST'} if args.scenario=='deny-write' else {'command':command,'description':'Bounded settlement fixture','mode':'async','detach':False})}}]},'finish_reason':None}]},{'id':'fixture-1','object':'chat.completion.chunk','choices':[{'index':0,'delta':{},'finish_reason':'tool_calls'}]}] + for value in values:self.wfile.write(('data: '+json.dumps(value)+'\n\n').encode()) + self.wfile.write(b'data: [DONE]\n\n') + else: + self.send_response(200);self.send_header('Content-Type','application/json');self.end_headers();self.wfile.write(json.dumps({'id':'fixture-1','object':'chat.completion','choices':[{'index':0,'message':{'role':'assistant','content':'Fixture complete.'},'finish_reason':'stop'}],'usage':{'prompt_tokens':10,'completion_tokens':3,'total_tokens':13}}).encode()) + server=http.server.HTTPServer(('127.0.0.1',0),Handler) + cleanup.callback(server.server_close) + threading.Thread(target=server.serve_forever,daemon=True).start() + cleanup.callback(server.shutdown) + env={'PATH':'/usr/bin:/bin','HOME':str(root/'home'),'XDG_CONFIG_HOME':str(root/'config'),'XDG_CACHE_HOME':str(root/'cache'),'XDG_DATA_HOME':str(root/'data'),'COPILOT_HOME':str(root/'copilot'),'COPILOT_CACHE_HOME':str(root/'copilot-cache'),'COPILOT_AUTO_UPDATE':'false','COPILOT_OFFLINE':'true','COPILOT_PROVIDER_BASE_URL':f'http://127.0.0.1:{server.server_port}','COPILOT_PROVIDER_TYPE':'openai','COPILOT_PROVIDER_MODEL_ID':'gpt-4.1','COPILOT_MODEL':'gpt-4.1','NO_COLOR':'1'} + for key in ('HOME','XDG_CONFIG_HOME','XDG_CACHE_HOME','XDG_DATA_HOME','COPILOT_HOME','COPILOT_CACHE_HOME'):pathlib.Path(env[key]).mkdir() + (root/'copilot/config.json').write_text(json.dumps({'trustedFolders':[],'disableAllHooks':True,'memory':False,'ide':{'autoConnect':False}})) + p=subprocess.Popen([binary,'--acp','--stdio','--no-auto-update','--disable-builtin-mcps','--no-remote','--no-remote-export','--no-bash-env'],env=env,cwd=root,stdin=subprocess.PIPE,stdout=subprocess.PIPE,stderr=subprocess.PIPE) + cleanup.callback(stop_process, p) + responses={};wire=[];permission_responses=[];buf={p.stdout:b'',p.stderr:b''};nextid=0;marker_at_prompt_result=False -def request(method,params): - global nextid - nextid+=1;n=nextid;p.stdin.write((json.dumps({'jsonrpc':'2.0','id':n,'method':method,'params':params})+'\n').encode());p.stdin.flush();deadline=time.monotonic()+30 - while n not in responses and time.monotonic() { + execFileSync("python3", [fileURLToPath(new URL("./probe-copilot-acp.test.py", import.meta.url))], { + encoding: "utf8", timeout: 15_000, stdio: "pipe", + }); +}); diff --git a/packages/paperclip-runner/scripts/probe-copilot-acp.test.py b/packages/paperclip-runner/scripts/probe-copilot-acp.test.py new file mode 100644 index 00000000000..6cf63e1550b --- /dev/null +++ b/packages/paperclip-runner/scripts/probe-copilot-acp.test.py @@ -0,0 +1,93 @@ +"""Credential-free failure injection for the offline probe's resource ownership.""" +import io +import json +import pathlib +import runpy +import tempfile +import unittest +from contextlib import ExitStack, redirect_stdout +from unittest.mock import Mock, patch + +SCRIPT = pathlib.Path(__file__).with_name("probe-copilot-acp.py") +DIGEST = "a9ff8babb10b7e443182ae96a8bc50a9c826ef1c773e1344c396eb5bf7f512c3" + + +class ProbeCleanupTests(unittest.TestCase): + def exercise(self, stage, broken_report=False, broken_stdin=False): + with tempfile.TemporaryDirectory(prefix="copilot-probe-test-") as temporary: + base = pathlib.Path(temporary) + package = base / "package" + package.mkdir() + (package / "package.json").write_text(json.dumps({"name": "@github/copilot-darwin-arm64", "version": "1.0.88"})) + (package / "copilot").write_bytes(b"fixture; never executed") + workspace = base / "workspace" + server = Mock(server_port=54321) + process = Mock() + process.stdout.fileno.return_value = 10 + process.stderr.fileno.return_value = 11 + if broken_stdin: + process.stdin.close.side_effect = BrokenPipeError("closed") + pending = [] + + def write(raw): + request = json.loads(raw) + if request["method"] != stage: + result = {"sessionId": "session-1"} if request["method"] == "session/new" else {"stopReason": "end_turn"} + pending.append((json.dumps({"jsonrpc": "2.0", "id": request["id"], "result": result}) + "\n").encode()) + process.stdin.write.side_effect = write + process.wait.return_value = 0 + clock = iter(range(0, 1000, 10)) + dumps = json.dumps + + def serialize(value, *args, **kwargs): + if broken_report and isinstance(value, dict) and value.get("schema") == "paperclip.copilot-acp-evidence/v1": + raise ValueError("report failed") + return dumps(value, *args, **kwargs) + + def make_workspace(**_kwargs): + workspace.mkdir() + return str(workspace) + + with ExitStack() as patches, redirect_stdout(io.StringIO()): + for target, value in [ + ("sys.argv", [str(SCRIPT), "--package-root", str(package)]), + ("hashlib.sha256", Mock(return_value=Mock(hexdigest=lambda: DIGEST))), + ("tempfile.mkdtemp", make_workspace), + ("http.server.HTTPServer", Mock(return_value=server)), + ("threading.Thread", Mock()), + ("subprocess.Popen", Mock(side_effect=OSError("spawn failed")) if stage == "spawn" else Mock(return_value=process)), + ("time.monotonic", lambda: next(clock)), + ("select.select", lambda *_args: ([process.stdout] if pending else [], [], [])), + ("os.read", lambda *_args: pending.pop(0)), + ("json.dumps", serialize), + ]: + patches.enter_context(patch(target, value)) + with self.assertRaisesRegex(OSError if stage == "spawn" else TimeoutError, "spawn failed" if stage == "spawn" else stage): + runpy.run_path(str(SCRIPT), run_name="__main__") + self.assertFalse(workspace.exists()) + server.shutdown.assert_called_once() + server.server_close.assert_called_once() + if stage != "spawn": + process.terminate.assert_called_once() + process.wait.assert_called_once_with(timeout=5) + process.stdout.close.assert_called_once() + process.stderr.close.assert_called_once() + + def test_initialize_timeout(self): + self.exercise("initialize") + + def test_session_creation_timeout(self): + self.exercise("session/new") + + def test_report_failure_preserves_original_failure_and_cleanup(self): + self.exercise("initialize", broken_report=True) + + def test_broken_input_pipe_does_not_skip_process_cleanup(self): + self.exercise("session/new", broken_stdin=True) + + def test_spawn_failure_closes_server_and_workspace(self): + self.exercise("spawn") + + +if __name__ == "__main__": + unittest.main() diff --git a/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.test.ts b/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.test.ts index 20aaa6a65c6..9618bfd93f8 100644 --- a/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.test.ts @@ -1,5 +1,6 @@ import { createHash } from "node:crypto"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import * as copilotEvents from "./copilot-events.js"; import { COPILOT_ACP_EVENT_METHOD } from "./copilot-events.js"; import { createCopilotProfileExtensionAdapter } from "./copilot-extension-adapter.js"; import { validateAcpxRichEvent } from "./profile-extensions.js"; @@ -63,6 +64,26 @@ describe("Copilot display extension adapter", () => { expect(await adapter.notification(COPILOT_ACP_EVENT_METHOD, { ...params("session.idle", {}), sessionId: "another" })).toEqual([]); }); + it("redacts sensitive field paths even when their plain values have no recognizable credential prefix", async () => { + // Exercise the display boundary independently from today's closed native + // field list, so a future projected field cannot bypass keyed redaction. + const normalize = vi.spyOn(copilotEvents, "normalizeCopilotSessionEvent").mockReturnValueOnce({ + kind: "usage", sourceMethod: COPILOT_ACP_EVENT_METHOD, sourceType: "assistant.usage", + sessionId: context.sessionId, turnId: context.turnId, + data: { connectionString: "plain-credential-value", credentials: { endpoint: "opaque-value" }, inputTokens: 42 }, + }); + try { + const events = await createCopilotProfileExtensionAdapter(context).notification(COPILOT_ACP_EVENT_METHOD, params("assistant.usage", {})); + events.forEach(validateAcpxRichEvent); + expect(events[0].payload.details).toEqual(expect.arrayContaining([ + { name: "connectionString", value: "[REDACTED]" }, + { name: "credentials.endpoint", value: "[REDACTED]" }, + { name: "inputTokens", value: "42" }, + ])); + expect(JSON.stringify(events)).not.toMatch(/plain-credential-value|opaque-value/); + } finally { normalize.mockRestore(); } + }); + it("emits compaction completion only after provider-reported success and keeps settlement notices nonterminal", async () => { const adapter = createCopilotProfileExtensionAdapter(context); const success = await adapter.notification(COPILOT_ACP_EVENT_METHOD, params("session.compaction_complete", { success: true, preCompactionTokens: 100, postCompactionTokens: 40, checkpointNumber: 1 })); diff --git a/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.ts b/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.ts index f5840dd086f..35019e00468 100644 --- a/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.ts +++ b/packages/paperclip-runner/src/drivers/acpx/copilot-extension-adapter.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import type { CanonicalProviderEvent } from "../../provider-events.js"; -import { redactPaperclipSemanticValue } from "../../semantic-tools/redaction.js"; +import { redactPaperclipSemanticValue, redactSemanticValue } from "../../semantic-tools/redaction.js"; import { COPILOT_ACP_EVENT_METHOD, normalizeCopilotSessionEvent, type CopilotSessionEvent } from "./copilot-events.js"; import type { AcpxProfileExtensionAdapter, AcpxProfileExtensionContext } from "./profile-extensions.js"; @@ -116,8 +116,11 @@ function detailFields(data: Record): Array<{ name: string; valu if (value !== null && typeof value === "object" && !Array.isArray(value)) { for (const [key, entry] of Object.entries(value)) visit(entry, name ? `${name}.${key}` : key); } else if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { - // Redact values individually: numeric token counters are not credentials. - details.push({ name: bounded(name, 160), value: bounded(String(value), 4000) }); + // Preserve the complete field path for sensitive-key redaction. The + // normalizer admits only known, typed counters/flags: their numeric token + // counts are not credentials, so they retain value-only redaction. + const safe = typeof value === "string" ? redactSemanticValue(value, name) : value; + details.push({ name: bounded(name, 160), value: bounded(String(safe), 4000) }); } }; visit(data, "");