diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4fccc47..45cac1c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,6 +56,9 @@ jobs: - name: Integration probe regression tests run: python3 tests/test-integration-probes.py + - name: Codex account wrapper regression tests + run: python3 tests/test-codex-account.py + - name: Integration test — proxy reverse forwarding run: ./tests/integration-proxy-forward.sh diff --git a/docs/codex-integration.md b/docs/codex-integration.md index 2285122..616e03e 100644 --- a/docs/codex-integration.md +++ b/docs/codex-integration.md @@ -16,6 +16,15 @@ unlocks GNOME Keyring, and then runs the real Codex binary. In keyring mode each launch gets a fresh private D-Bus session, so the wrapper asks for the guest keyring password every time before Codex starts. +The wrapper also supplies `-c 'cli_auth_credentials_store="keyring"'`. +In Codex 0.153.0 and 0.154.0, this override prevents implicit reuse of a desktop +app-server whose D-Bus session may have an unusable keyring. Terminal sign-in +then uses the session unlocked by the wrapper, including through `codex-yolo`. +Caller arguments follow this default and retain their precedence; explicitly +selecting a remote app-server still selects that server and its auth session. +API-key mode remains a passthrough. This daemon-selection behavior is +version-dependent and should be rechecked when updating Codex. + The in-guest `codex-yolo` shortcut routes through the same wrapper, so it works in either auth mode. Running the bare `codex` binary from `coop shell` does not: it has no D-Bus session, and `keyring` credential storage has no diff --git a/docs/testing.md b/docs/testing.md index 4183658..a5c34f0 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -54,6 +54,22 @@ Linux CI runs them. The full VM suite additionally checks these probes against real guests. A host FORWARD policy other than ACCEPT still causes an explicit skip of the routed guest-isolation probe, since it would mask the coop rule. +Run `python3 tests/test-codex-account.py` for the account wrapper's argument, +login/logout, API-key passthrough, and `codex-yolo` regressions (also in Linux +CI). To additionally test implicit daemon reuse with a real Linux Codex binary: + +```bash +COOP_TEST_CODEX="$(command -v codex)" python3 tests/test-codex-account.py +``` + +This requires `dbus-run-session`, `gnome-keyring-daemon`, `secret-tool`, and +`strace`. It uses temporary homes and disposable keyring passwords, starts a +real app-server on a separate unusable keyring session, and observes terminal +socket connections. It checks that sign-in is reached without reusing that +server and that removing the wrapper override restores reuse. No account login +or real tokens are needed. Run it when upgrading Codex: daemon selection is +version-dependent. This opt-in test does not replace either VM backend gate. + The full Codex update tests install native release `0.153.0` before running `codex update` as the guest user, and require the installed version to change. They compare the actual `config.toml` contents across host updates, self-updates, diff --git a/scripts/guest/codex-account.sh b/scripts/guest/codex-account.sh index eb116e0..167f26b 100644 --- a/scripts/guest/codex-account.sh +++ b/scripts/guest/codex-account.sh @@ -171,6 +171,9 @@ else export COOP_CODEX_ACCOUNT_UNLOCKED=1 fi -exec "$CODEX_BIN" "$@" +# Codex 0.154.0 can reuse a desktop daemon on another D-Bus session when +# there are no explicit config overrides. Keep terminal auth on the keyring +# we just unlocked. Prepend the default so caller overrides retain precedence. +exec "$CODEX_BIN" -c 'cli_auth_credentials_store="keyring"' "$@" CODEXACCOUNTEOF chmod 755 /usr/local/bin/codex-account diff --git a/tests/test-codex-account.py b/tests/test-codex-account.py new file mode 100644 index 0000000..39c1022 --- /dev/null +++ b/tests/test-codex-account.py @@ -0,0 +1,209 @@ +#!/usr/bin/env python3 +"""Wrapper regressions; set COOP_TEST_CODEX to also test a real Linux Codex CLI.""" +import json +import os +from pathlib import Path +import pty +import select +import shutil +import signal +import struct +import subprocess +import sys +import tempfile +import time +import unittest +import fcntl +import termios + +ROOT = Path(__file__).resolve().parent.parent +SOURCE = (ROOT / 'scripts/guest/codex-account.sh').read_text() +WRAPPER = SOURCE.split("<<'CODEXACCOUNTEOF'\n", 1)[1].split('\nCODEXACCOUNTEOF', 1)[0] +OVERRIDE = ['-c', 'cli_auth_credentials_store="keyring"'] + + +def executable(path, source): + path.write_text(source) + path.chmod(0o755) + + +def isolated_env(root): + # Do not let host credentials or launch overrides affect the fixture. + env = {k: v for k, v in os.environ.items() + if not k.startswith(('CODEX_', 'COOP_CODEX_', 'OPENAI_', 'XDG_', 'DBUS_', 'GNOME_KEYRING'))} + env.update(HOME=str(root), XDG_DATA_HOME=str(root / 'data'), + XDG_RUNTIME_DIR=str(root / 'run'), TERM='xterm-256color') + (root / '.codex').mkdir() + (root / 'run').mkdir(mode=0o700) + return env + + +def stop(process): + # dbus-run-session may exit before Codex; waiting only for the launcher + # races children still writing into the temporary home. + for sig in [signal.SIGTERM, signal.SIGKILL]: + try: + os.killpg(process.pid, sig) + except ProcessLookupError: + process.wait(timeout=5) + return + deadline = time.monotonic() + 5 + while time.monotonic() < deadline: + process.poll() + try: + os.killpg(process.pid, 0) + except ProcessLookupError: + process.wait(timeout=5) + return + time.sleep(0.05) + raise RuntimeError(f'fixture process group {process.pid} did not exit') + + +class WrapperTests(unittest.TestCase): + def test_cleanup_waits_for_launcher_children(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + child = root / 'child.py' + child.write_text(''' +import signal, sys, time +from pathlib import Path +def finish(*args): + time.sleep(0.2) + Path(sys.argv[1], 'finished').touch() + sys.exit(0) +signal.signal(signal.SIGTERM, finish) +Path(sys.argv[1], 'ready').touch() +time.sleep(60) +''') + parent = subprocess.Popen([sys.executable, '-c', ''' +import subprocess, sys, time +subprocess.Popen([sys.executable, sys.argv[1], sys.argv[2]]) +time.sleep(60) +''', str(child), str(root)], start_new_session=True) + try: + deadline = time.monotonic() + 5 + while not (root / 'ready').exists() and time.monotonic() < deadline: + time.sleep(0.01) + self.assertTrue((root / 'ready').exists()) + finally: + stop(parent) + self.assertTrue((root / 'finished').exists(), 'cleanup returned before child exit') + + def test_arguments_and_passthrough(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + env = isolated_env(root) + binary = root / 'codex' + executable(binary, '#!/usr/bin/env python3\nimport json,sys\nprint(json.dumps(sys.argv[1:]))\nsys.exit(23)\n') + wrapper = root / 'codex-account' + executable(wrapper, WRAPPER.replace('/usr/local/bin/codex', str(binary))) + for tool in ['secret-tool', 'gnome-keyring-daemon']: + executable(root / tool, '#!/bin/sh\nexit 0\n') + env['PATH'] = str(root) + ':' + env['PATH'] + env.update(COOP_CODEX_ACCOUNT_DBUS='1', COOP_CODEX_ACCOUNT_UNLOCKED='1') + config = root / '.codex/config.toml' + for keyring in [False, True]: + config.write_text('cli_auth_credentials_store = "keyring"\n' if keyring else '') + for args in [[], ['login', '--device-auth'], ['logout'], + ['--', 'a prompt with spaces; $(false)'], + ['-c', 'model="example"', '--model', 'explicit'], + ['-c', 'cli_auth_credentials_store="file"'], + ['--remote', 'unix:///explicit.sock']]: + with self.subTest(keyring=keyring, args=args): + result = subprocess.run([str(wrapper), *args], env=env, + capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 23, result.stderr) + self.assertEqual(json.loads(result.stdout), (OVERRIDE if keyring else []) + args) + # Exercise the actual provisioned yolo command, including its bypass flag. + lima = (ROOT / 'src/lima.rs').read_text() + yolo = lima.split("cat > /usr/local/bin/codex-yolo <<'YOLOEOF'\n", 1)[1].split('\nYOLOEOF', 1)[0] + shortcut = root / 'codex-yolo' + executable(shortcut, yolo.replace('/usr/local/bin/codex-account', str(wrapper))) + result = subprocess.run([str(shortcut), 'hello world'], env=env, + capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 23, result.stderr) + self.assertEqual(json.loads(result.stdout), OVERRIDE + + ['--dangerously-bypass-approvals-and-sandbox', 'hello world']) + + +@unittest.skipUnless(os.environ.get('COOP_TEST_CODEX'), 'set COOP_TEST_CODEX for real daemon regression') +class RealDaemonTests(unittest.TestCase): + def test_terminal_avoids_daemon_with_unusable_keyring(self): + binary = str(Path(os.environ['COOP_TEST_CODEX']).resolve()) + for tool in ['dbus-run-session', 'gnome-keyring-daemon', 'secret-tool', 'strace']: + self.assertIsNotNone(shutil.which(tool), f'missing prerequisite: {tool}') + print(subprocess.check_output([binary, '--version'], text=True).strip(), flush=True) + with tempfile.TemporaryDirectory(prefix='c481-') as directory: + root = Path(directory) + env = isolated_env(root) + # Disable unrelated plugin downloads without a CLI override, which would + # itself prevent the control launch from reusing the daemon. + (root / '.codex/config.toml').write_text( + 'cli_auth_credentials_store = "keyring"\n[features]\nplugins = false\n') + wrapper = root / 'codex-account' + executable(wrapper, WRAPPER.replace('/usr/local/bin/codex', binary)) + socket = root / '.codex/app-server-control/app-server-control.sock' + # The existing server gets a different bus and an empty keyring directory. + daemon_env = {**env, 'XDG_DATA_HOME': str(root / 'desktop-data')} + with (root / 'daemon.log').open('w') as log: + daemon = subprocess.Popen(['dbus-run-session', '--', 'bash', '-c', ''' + printf '%s' "$DBUS_SESSION_BUS_ADDRESS" > "$HOME/desktop-bus" + exec "$1" app-server --listen unix:// + ''', 'bash', binary], env=daemon_env, stdout=log, stderr=log, start_new_session=True) + try: + deadline = time.monotonic() + 20 + while not socket.exists() and daemon.poll() is None and time.monotonic() < deadline: + time.sleep(0.05) + self.assertTrue(socket.exists(), (root / 'daemon.log').read_text()) + bad_env = {**daemon_env, 'DBUS_SESSION_BUS_ADDRESS': (root / 'desktop-bus').read_text()} + probe = subprocess.run(['timeout', '5', 'secret-tool', 'store', '--label=probe', + 'service', 'coop-regression'], input='disposable', text=True, + capture_output=True, env=bad_env, timeout=10) + self.assertNotEqual(probe.returncode, 0, 'desktop keyring unexpectedly writable') + # Positive witness: removing only the fix must connect to that daemon. + mutant = root / 'codex-account-mutant' + executable(mutant, wrapper.read_text().replace( + "-c 'cli_auth_credentials_store=\"keyring\"' ", '')) + old_trace = self.launch(root, env, mutant, 'old') + self.assertIn(str(socket), old_trace, 'control never reused the existing daemon') + new_trace = self.launch(root, env, wrapper, 'new') + self.assertNotIn(str(socket), new_trace, 'terminal reused the desktop daemon') + self.assertIsNone(daemon.poll(), 'test must leave the existing server running') + self.assertFalse((root / '.codex/auth.json').exists()) + finally: + stop(daemon) + + def launch(self, root, env, wrapper, label): + trace = root / (label + '.trace') + master, slave = pty.openpty() + fcntl.ioctl(slave, termios.TIOCSWINSZ, struct.pack('HHHH', 40, 120, 0, 0)) + process = subprocess.Popen(['strace', '-f', '-e', 'connect', '-s', '256', '-o', str(trace), + str(wrapper)], env=env, cwd=root, stdin=slave, stdout=slave, + stderr=slave, start_new_session=True) + os.close(slave) + output = b'' + deadline = time.monotonic() + 30 + try: + while time.monotonic() < deadline and process.poll() is None: + if not select.select([master], [], [], 0.1)[0]: + continue + try: + chunk = os.read(master, 65536) + except OSError: + break + output += chunk + if b'\x1b[6n' in chunk: + os.write(master, b'\x1b[1;1R') + if b'keyring password: ' in chunk: + os.write(master, b'coop-test-password\n') + if b'Sign in with ChatGPT' in output: + break + self.assertIn(b'Sign in with ChatGPT', output, output.decode(errors='replace')) + return trace.read_text() + finally: + stop(process) + os.close(master) + + +if __name__ == '__main__': + unittest.main()