diff --git a/docs/design/github-channel-gh-auth.md b/docs/design/github-channel-gh-auth.md new file mode 100644 index 00000000000..d3e2d072173 --- /dev/null +++ b/docs/design/github-channel-gh-auth.md @@ -0,0 +1,62 @@ +# GitHub Channel local `gh` authentication + +## Problem + +The GitHub Channel currently requires a classic personal access token in every configuration. This prevents Web Shell users from creating a channel that reuses the GitHub CLI authentication already available to the daemon host through `gh auth login`. + +The separate Web Shell pull-request integration already relies on the daemon host's `gh` installation and authentication, but the Channel adapter passes only its configured `token` to Octokit. + +## Proposed behavior + +- Keep an explicitly configured channel token as the highest-priority credential. +- Add an explicit `useLocalGh` opt-in for reusing the daemon host's account-wide GitHub CLI credential. +- When the token is absent and `useLocalGh` is enabled, resolve a token by running `gh auth token --hostname ` in the Channel worker. +- Reject configurations that provide neither an explicit token nor the opt-in. +- Use `github.com` as the local `gh` authentication hostname for the default `https://api.github.com` API URL. +- Derive the hostname from a configured GitHub Enterprise `baseUrl`. +- Require `baseUrl` to use HTTPS before resolving a daemon host credential through local `gh` authentication. +- Fail Channel startup with actionable diagnostics when `gh` is unavailable or the selected host is not authenticated. +- Never persist or expose the token returned by `gh`. + +## Changes + +### GitHub Channel plugin + +Make the managed `token` secret optional, remove it from startup-required fields, and add a `useLocalGh` boolean. Update the descriptions to explain that an explicit classic PAT overrides local GitHub CLI authentication. The plugin's management descriptor validates the resolved configuration during managed upserts and rejects one that provides neither a token nor the opt-in, so the daemon mutation boundary keeps the immediate save-time rejection the required token provided before, while `connect()` still rejects configurations whose runtime credential cannot be resolved. + +### GitHub Channel adapter + +Resolve credentials during `connect()` before constructing Octokit. Use `execFile` without a shell, a bounded timeout, and a bounded output buffer. Pass the selected hostname as a separate argument. The Channel worker already inherits the daemon's `PATH`, `HOME`, and related environment, so `gh` reads the daemon host's existing login. + +### Web Shell + +The descriptor-driven editor already supports optional secret and boolean fields. Expose `useLocalGh` and require either a preserved/non-empty token or the explicit opt-in before saving. An existing PAT can be cleared only when local `gh` authentication is selected. Update localized field text accordingly. + +### Documentation + +Document local `gh auth login` as an explicit opt-in and explicit PAT configuration as an override. Warn that the local credential is account-wide and preserve the recommendation to use a separate bot account because the authenticated account cannot trigger its own channel. + +## Files affected + +- `packages/channels/github/src/index.ts` +- `packages/channels/github/src/GithubAdapter.ts` +- `packages/channels/github/src/GithubAdapter.test.ts` +- `packages/cli/src/commands/channel/channel-registry.test.ts` +- `packages/web-shell/client/components/channels/channel-editor-state.ts` +- `packages/web-shell/client/components/channels/channel-editor-state.test.ts` +- `packages/web-shell/client/components/channels/ChannelEditorDialog.tsx` +- `packages/web-shell/client/e2e/visuals/screenshots.spec.ts` +- `packages/web-shell/client/i18n.tsx` +- `docs/users/features/channels/github.md` +- `docs/design/github-channel-gh-auth.md` + +## Scope boundaries + +- No automatic login or interactive `gh auth login` invocation. +- No GitHub App or fine-grained PAT support. +- No shared cross-package GitHub credential abstraction. +- No change to GitLab Channel authentication. + +## Security considerations + +The resolved token stays in memory and is passed only to Octokit. It is not written into settings or logs. The subprocess uses fixed arguments and no shell. Existing sender-policy and self-authored-comment protections remain unchanged. diff --git a/docs/users/features/channels/github.md b/docs/users/features/channels/github.md index 73523f7156d..7d433e0c080 100644 --- a/docs/users/features/channels/github.md +++ b/docs/users/features/channels/github.md @@ -4,17 +4,31 @@ This guide covers setting up a Qwen Code channel that monitors GitHub notificati ## Prerequisites -- A GitHub account for the channel. Use a dedicated bot account when the PAT - owner also needs to operate the channel. -- A GitHub Personal Access Token (PAT) with `notifications` and `public_repo` (or `repo`) scopes +- A GitHub account authenticated with the permissions needed to read notifications and post comments +- The [GitHub CLI](https://cli.github.com/) installed on the host running Qwen Code when using local `gh` authentication -## Creating a Token +Use a dedicated bot account when the authenticated account also needs to operate the channel. GitHub does not generate a usable notification for the account's own activity, and the adapter ignores its own comments to prevent reply loops. -1. Go to **Settings → Developer settings → Personal access tokens → Tokens (classic)** -2. Generate a token with these scopes: - - **notifications** — read notification threads - - **public_repo** (or **repo** for private repos) — post comments -3. Save the token securely as an environment variable +## Authentication + +To reuse the GitHub CLI login on the Qwen Code host, authenticate `gh` and explicitly set `useLocalGh: true` in the channel configuration: + +```bash +gh auth login +``` + +Local `gh` authentication is account-wide and may expose notifications from every repository visible to that GitHub account. Enable it only when the workspace operator is trusted to use that account. Otherwise, configure a dedicated PAT. + +For GitHub Enterprise Server, authenticate the same host used by `baseUrl`: + +```bash +gh auth login --hostname github.example.com +``` + +You can instead configure a classic personal access token (PAT). An explicit `token` overrides local `gh` authentication. The PAT needs these scopes: + +- **notifications** — read notification threads +- **public_repo** (or **repo** for private repos) — post comments ## Configuration @@ -25,7 +39,7 @@ Add the channel to `~/.qwen/settings.json`: "channels": { "my-github": { "type": "github", - "token": "$GITHUB_TOKEN", + "useLocalGh": true, "pollInterval": 60000, "reasonFilter": ["mention", "review_requested", "assign"], "senderPolicy": "allowlist", @@ -42,18 +56,13 @@ Add the channel to `~/.qwen/settings.json`: } ``` -Set the token as an environment variable: +To override local `gh` authentication with a PAT, add `"token": "$GITHUB_TOKEN"` to the channel and set the environment variable before starting Qwen Code: ```bash export GITHUB_TOKEN="ghp_your_token_here" ``` -The PAT owner cannot trigger its own channel: GitHub self-activity does not -provide a usable notification, and the adapter intentionally ignores its own -comments to prevent reply loops. If the PAT owner needs to operate the channel, -use a separate bot-owned PAT and put only operator accounts in `allowedUsers`. -Startup rejects an allowlist containing only the PAT owner and warns when the -PAT owner appears alongside other operators. +The authenticated account cannot trigger its own channel. If that account needs to operate the channel, authenticate a separate bot account and put only operator accounts in `allowedUsers`. Startup rejects an allowlist containing only the authenticated account and warns when it appears alongside other operators. ### GitHub Enterprise @@ -65,11 +74,14 @@ For GitHub Enterprise Server, set `baseUrl`: } ``` +Local `gh` authentication requires an HTTPS `baseUrl` so the daemon host credential cannot be sent over plaintext HTTP. + ## Configuration Options | Option | Default | Description | | ------------------------- | ------------------------ | --------------------------------------------------------------------------------------------- | -| `token` | (required) | Classic PAT with `notifications` scope | +| `token` | unset | Optional classic PAT with `notifications` scope; overrides local `gh` authentication | +| `useLocalGh` | `false` | Explicitly reuse the daemon host's account-wide GitHub CLI authentication | | `pollInterval` | `60000` | Poll interval in ms | | `baseUrl` | `https://api.github.com` | API base URL (for GHE) | | `groupPolicy` | `"disabled"` | Must be `"open"` for notifications to flow | @@ -141,7 +153,7 @@ not retried automatically because GitHub may have created the comment. - If a user marks a notification as read on github.com before the bot's poll cycle, the bot will not process it. - The bot does not read comments before the current polling window; `author` and `comment` notifications may aggregate up to 20 comments from that window. - Inline PR review comments and review summary bodies are not enumerated; only issue/PR comments are processed. -- Requires a classic PAT with `notifications` scope. Fine-grained PATs do not support the notifications API. +- The selected credential must support the Notifications API. Fine-grained PATs do not support it; use local `gh` authentication or a classic PAT with `notifications` scope. ## Starting the Channel diff --git a/packages/channels/base/src/types.ts b/packages/channels/base/src/types.ts index f5338cc5646..b528e8e26b9 100644 --- a/packages/channels/base/src/types.ts +++ b/packages/channels/base/src/types.ts @@ -401,6 +401,15 @@ export interface ChannelConfigFieldDescriptor { export interface ChannelManagementDescriptor { fields: readonly ChannelConfigFieldDescriptor[]; + + /** + * Cross-field validation applied to the resolved config during managed + * upserts, after secret updates. Return an error message to reject the + * update, or undefined to accept it. + */ + validateConfig?: ( + config: Readonly>, + ) => string | undefined; } /** diff --git a/packages/channels/github/src/GithubAdapter.test.ts b/packages/channels/github/src/GithubAdapter.test.ts index 6fd8990142e..5bbd02a753f 100644 --- a/packages/channels/github/src/GithubAdapter.test.ts +++ b/packages/channels/github/src/GithubAdapter.test.ts @@ -26,6 +26,12 @@ import { type Envelope, } from '@qwen-code/channel-base'; +const mockExecFile = vi.hoisted(() => vi.fn()); + +vi.mock('node:child_process', () => ({ + execFile: mockExecFile, +})); + vi.mock('@octokit/rest', () => { const mockOctokit = { rest: { @@ -68,11 +74,12 @@ vi.mock('@qwen-code/channel-base', async (importOriginal) => { import { GithubChannel } from './GithubAdapter.js'; -const mockOctokit = ( - (await import('@octokit/rest')) as unknown as { - __mockOctokit: Record; - } -).__mockOctokit as { +const octokitModule = (await import('@octokit/rest')) as unknown as { + Octokit: Mock; + __mockOctokit: Record; +}; +const mockOctokitConstructor = octokitModule.Octokit; +const mockOctokit = octokitModule.__mockOctokit as { rest: { users: { getAuthenticated: ReturnType; @@ -323,6 +330,7 @@ describe('GithubChannel', () => { rmSync(process.env.QWEN_HOME!, { recursive: true, force: true }); if (savedQwenHome === undefined) delete process.env.QWEN_HOME; else process.env.QWEN_HOME = savedQwenHome; + vi.unstubAllEnvs(); }); async function initWithoutLoop(configOverrides?: Record) { @@ -347,11 +355,674 @@ describe('GithubChannel', () => { describe('connect', () => { it('resolves bot username', async () => { mockOctokit.paginate.mockResolvedValue([]); + const stderr = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + try { + await channel.connect(); + expect(mockOctokit.rest.users.getAuthenticated).toHaveBeenCalled(); + expect(stderr).toHaveBeenCalledWith( + '[Channel:test-github] using configured token\n', + ); + expect(stderr).toHaveBeenCalledWith( + '[Channel:test-github] authenticated as "test-bot"\n', + ); + } finally { + channel.disconnect(); + stderr.mockRestore(); + } + }); + + it('sanitizes the authenticated login in the stderr audit line', async () => { + mockOctokit.rest.users.getAuthenticated.mockResolvedValue({ + data: { id: 99999, login: 'bot\nforged-line' }, + }); + mockOctokit.paginate.mockResolvedValue([]); + const stderr = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + try { + await channel.connect(); + expect(stderr).toHaveBeenCalledWith( + '[Channel:test-github] authenticated as "bot\\nforged-line"\n', + ); + } finally { + channel.disconnect(); + stderr.mockRestore(); + } + }); + + it('requires explicit opt-in before using local gh authentication', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '' }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'configure a GitHub token or enable local GitHub CLI authentication', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + }); + + it('rejects a whitespace-only token', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: ' ' }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'configure a GitHub token or enable local GitHub CLI authentication', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + }); + + it('rejects a quoted useLocalGh value from hand-edited settings', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: 'true' }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + '[Channel:test-github] useLocalGh must be a boolean.', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + }); + + it('rejects a non-boolean useLocalGh even when a token is configured', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: 'test-token', useLocalGh: 'true' }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'useLocalGh must be a boolean', + ); + expect(mockOctokitConstructor).not.toHaveBeenCalled(); + }); + + it('falls back to local gh for a whitespace-only token', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string) => void, + ) => callback(null, 'local-gh-token\n'), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: ' ', useLocalGh: true }), + makeBridge(), + ); + mockOctokit.paginate.mockResolvedValue([]); + await channel.connect(); - expect(mockOctokit.rest.users.getAuthenticated).toHaveBeenCalled(); + + expect(mockExecFile).toHaveBeenCalled(); + expect(mockOctokitConstructor).toHaveBeenCalledWith( + expect.objectContaining({ auth: 'local-gh-token' }), + ); channel.disconnect(); }); + it('uses local gh authentication when explicitly enabled', async () => { + vi.stubEnv('GH_TOKEN', 'environment-token'); + vi.stubEnv('GITHUB_TOKEN', 'environment-token'); + vi.stubEnv('GH_ENTERPRISE_TOKEN', 'environment-token'); + vi.stubEnv('GITHUB_ENTERPRISE_TOKEN', 'environment-token'); + vi.stubEnv('GH_CONFIG_DIR', '/tmp/test-gh-config'); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string) => void, + ) => callback(null, 'local-gh-token\n'), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + mockOctokit.paginate.mockResolvedValue([]); + const stderr = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + + try { + await channel.connect(); + + expect(mockExecFile).toHaveBeenCalledWith( + 'gh', + ['auth', 'token', '--hostname', 'github.com'], + expect.objectContaining({ + encoding: 'utf8', + maxBuffer: 64 * 1024, + timeout: 10_000, + windowsHide: true, + env: expect.objectContaining({ + GH_CONFIG_DIR: '/tmp/test-gh-config', + }), + }), + expect.any(Function), + ); + const options = mockExecFile.mock.calls[0]?.[2] as { + env: NodeJS.ProcessEnv; + }; + expect(options.env).not.toHaveProperty('GH_TOKEN'); + expect(options.env).not.toHaveProperty('GITHUB_TOKEN'); + expect(options.env).not.toHaveProperty('GH_ENTERPRISE_TOKEN'); + expect(options.env).not.toHaveProperty('GITHUB_ENTERPRISE_TOKEN'); + expect(options.env['PATH']).toBe(process.env['PATH']); + expect(mockOctokitConstructor).toHaveBeenCalledWith( + expect.objectContaining({ auth: 'local-gh-token' }), + ); + expect(stderr).not.toHaveBeenCalledWith( + expect.stringContaining('local-gh-token'), + ); + } finally { + channel.disconnect(); + stderr.mockRestore(); + } + }); + + it('uses the enterprise host for local gh authentication', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string) => void, + ) => callback(null, 'enterprise-token\n'), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: '', + useLocalGh: true, + baseUrl: 'https://ghe.example.com:8443/api/v3', + }), + makeBridge(), + ); + mockOctokit.paginate.mockResolvedValue([]); + const stderr = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + + try { + await channel.connect(); + + expect(mockExecFile).toHaveBeenCalledWith( + 'gh', + ['auth', 'token', '--hostname', 'ghe.example.com'], + expect.any(Object), + expect.any(Function), + ); + expect(stderr).toHaveBeenCalledWith( + '[Channel:test-github] using local gh credential for ghe.example.com\n', + ); + expect(stderr).not.toHaveBeenCalledWith( + expect.stringContaining('enterprise-token'), + ); + } finally { + channel.disconnect(); + stderr.mockRestore(); + } + }); + + it('rejects an insecure API URL before resolving local gh credentials', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: '', + useLocalGh: true, + baseUrl: 'http://api.github.com', + }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'local GitHub CLI authentication requires an HTTPS baseUrl', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).not.toHaveBeenCalled(); + }); + + it('reports a malformed baseUrl before resolving local gh credentials', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: '', + useLocalGh: true, + baseUrl: 'ghe.example.com/api/v3', + }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + '[Channel:test-github] baseUrl is not a valid URL: ghe.example.com/api/v3', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).not.toHaveBeenCalled(); + }); + + it('reports a scheme-less baseUrl with a port as malformed', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: '', + useLocalGh: true, + baseUrl: 'ghe.example.com:8443/api/v3', + }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + '[Channel:test-github] baseUrl is not a valid URL: ghe.example.com:8443/api/v3', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).not.toHaveBeenCalled(); + }); + + it('rejects a baseUrl hostname that begins with a dash before spawning gh', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: '', + useLocalGh: true, + baseUrl: 'https://--evil/api/v3', + }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + '[Channel:test-github] baseUrl hostname is invalid: --evil', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).not.toHaveBeenCalled(); + }); + + it('rejects a baseUrl hostname outside the gh hostname allowlist', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: '', + useLocalGh: true, + baseUrl: 'https://ghe.example_company.com/api/v3', + }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + '[Channel:test-github] baseUrl hostname is invalid: ghe.example_company.com', + ); + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).not.toHaveBeenCalled(); + }); + + it('preserves explicit token support for an HTTP base URL', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ + token: 'test-token', + useLocalGh: true, + baseUrl: 'http://ghe.example.com/api/v3', + }), + makeBridge(), + ); + mockOctokit.paginate.mockResolvedValue([]); + + await channel.connect(); + + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).toHaveBeenCalledWith( + expect.objectContaining({ + auth: 'test-token', + baseUrl: 'http://ghe.example.com/api/v3', + }), + ); + channel.disconnect(); + }); + + it('reports an empty token returned by GitHub CLI', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string) => void, + ) => callback(null, ' \n'), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'GitHub CLI returned an empty token for github.com', + ); + }); + + it('prefers an explicit token over enabled local gh authentication', async () => { + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: 'test-token', useLocalGh: true }), + makeBridge(), + ); + mockOctokit.paginate.mockResolvedValue([]); + + await channel.connect(); + + expect(mockExecFile).not.toHaveBeenCalled(); + expect(mockOctokitConstructor).toHaveBeenCalledWith( + expect.objectContaining({ auth: 'test-token' }), + ); + channel.disconnect(); + }); + + it('reports when GitHub CLI is unavailable', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: NodeJS.ErrnoException, stdout: string) => void, + ) => + callback( + Object.assign(new Error('secret missing-cli failure'), { + code: 'ENOENT', + }), + '', + ), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'GitHub CLI (gh) is not installed on the daemon host', + ); + await expect(channel.connect()).rejects.not.toThrow( + 'secret missing-cli failure', + ); + }); + + it('reports when the selected gh host is not authenticated', async () => { + vi.stubEnv('GH_CONFIG_DIR', ''); + vi.stubEnv('XDG_CONFIG_HOME', ''); + vi.stubEnv('APPDATA', ''); + vi.stubEnv('HOME', '/home/test-user'); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error & { code: number }, stdout: string) => void, + ) => + callback(Object.assign(new Error('secret stderr'), { code: 1 }), ''), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'gh auth login --hostname github.com', + ); + await expect(channel.connect()).rejects.toThrow( + 'gh config dir: /home/test-user/.config/gh', + ); + await expect(channel.connect()).rejects.not.toThrow('secret stderr'); + }); + + it('names the gh config dir when the host is not authenticated', async () => { + vi.stubEnv('GH_CONFIG_DIR', '/tmp/test-gh-config'); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error & { code: number }, stdout: string) => void, + ) => callback(Object.assign(new Error('exit 1'), { code: 1 }), ''), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'gh config dir: /tmp/test-gh-config', + ); + }); + + it('prefers XDG_CONFIG_HOME over HOME in the gh config dir hint', async () => { + vi.stubEnv('GH_CONFIG_DIR', ''); + vi.stubEnv('XDG_CONFIG_HOME', '/tmp/test-xdg-config'); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error & { code: number }, stdout: string) => void, + ) => callback(Object.assign(new Error('exit 1'), { code: 1 }), ''), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'gh config dir: /tmp/test-xdg-config/gh', + ); + }); + + it('reports an unknown gh config dir when no config source is available', async () => { + vi.stubEnv('GH_CONFIG_DIR', ''); + vi.stubEnv('XDG_CONFIG_HOME', ''); + vi.stubEnv('APPDATA', ''); + vi.stubEnv('HOME', ''); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error & { code: number }, stdout: string) => void, + ) => callback(Object.assign(new Error('exit 1'), { code: 1 }), ''), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow('gh config dir: unknown'); + }); + + it('prefers the Windows AppData gh config dir on win32', async () => { + vi.stubEnv('GH_CONFIG_DIR', ''); + vi.stubEnv('XDG_CONFIG_HOME', ''); + vi.stubEnv('APPDATA', 'C:\\Users\\test\\AppData\\Roaming'); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error & { code: number }, stdout: string) => void, + ) => callback(Object.assign(new Error('exit 1'), { code: 1 }), ''), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + const platform = vi + .spyOn(process, 'platform', 'get') + .mockReturnValue('win32'); + + try { + await expect(channel.connect()).rejects.toThrow( + 'gh config dir: C:\\Users\\test\\AppData\\Roaming\\GitHub CLI', + ); + } finally { + platform.mockRestore(); + } + }); + + it('falls back to HOME when APPDATA is unset on win32', async () => { + vi.stubEnv('GH_CONFIG_DIR', ''); + vi.stubEnv('XDG_CONFIG_HOME', ''); + vi.stubEnv('APPDATA', ''); + vi.stubEnv('HOME', '/home/test-user'); + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error & { code: number }, stdout: string) => void, + ) => callback(Object.assign(new Error('exit 1'), { code: 1 }), ''), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + const platform = vi + .spyOn(process, 'platform', 'get') + .mockReturnValue('win32'); + + try { + await expect(channel.connect()).rejects.toThrow( + 'gh config dir: /home/test-user/.config/gh', + ); + } finally { + platform.mockRestore(); + } + }); + + it('surfaces bounded gh stderr in the authentication failure', async () => { + const rawStderr = `\u001b[2Jsecret${'x'.repeat(600)}`; + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: ( + error: Error & { code: number }, + stdout: string, + stderr: string, + ) => void, + ) => + callback( + Object.assign(new Error('exit 1'), { code: 1 }), + '', + rawStderr, + ), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'gh auth login --hostname github.com', + ); + const error = (await channel + .connect() + .catch((err: unknown) => err)) as Error; + expect(error).toBeInstanceOf(Error); + expect(error.message).not.toContain('\u001b'); + const hint = error.message.split(' gh stderr: ')[1] ?? ''; + expect(hint).toContain('[2Jsecret'); + expect(Array.from(hint).length).toBeLessThanOrEqual(256); + }); + + it('reports when the GitHub CLI authentication lookup times out', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: ( + error: Error & { killed: boolean }, + stdout: string, + ) => void, + ) => + callback( + Object.assign(new Error('secret timeout failure'), { + killed: true, + }), + '', + ), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'authentication lookup for github.com timed out after 10 seconds', + ); + await expect(channel.connect()).rejects.not.toThrow( + 'secret timeout failure', + ); + }); + + it('treats a killed lookup that also exited as a timeout', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: ( + error: Error & { code: number; killed: boolean }, + stdout: string, + ) => void, + ) => + callback( + Object.assign(new Error('exit 1'), { code: 1, killed: true }), + '', + ), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'authentication lookup for github.com timed out after 10 seconds', + ); + }); + + it('reports when GitHub CLI authentication cannot execute', async () => { + mockExecFile.mockImplementation( + ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: NodeJS.ErrnoException, stdout: string) => void, + ) => + callback( + Object.assign(new Error('secret failure'), { code: 'EACCES' }), + '', + ), + ); + channel = new TestableGithubChannel( + 'test-github', + makeConfig({ token: '', useLocalGh: true }), + makeBridge(), + ); + + await expect(channel.connect()).rejects.toThrow( + 'authentication lookup for github.com failed to execute', + ); + await expect(channel.connect()).rejects.not.toThrow('secret failure'); + }); + it('throws when bot identity fails', async () => { mockOctokit.rest.users.getAuthenticated.mockRejectedValue( new Error('bad token'), @@ -3736,6 +4407,51 @@ describe('GithubChannel', () => { const { plugin } = await import('./index.js'); expect(plugin.defaultSessionScope).toBe('chat_thread'); }); + + it('allows explicit local gh authentication without a configured token', async () => { + const { plugin } = await import('./index.js'); + const tokenField = plugin.management?.fields.find( + (field) => field.key === 'token', + ); + const localGhField = plugin.management?.fields.find( + (field) => field.key === 'useLocalGh', + ); + expect(plugin.requiredConfigFields).toBeUndefined(); + expect(tokenField).toMatchObject({ kind: 'secret' }); + expect(tokenField).not.toHaveProperty('required'); + expect(localGhField).toMatchObject({ kind: 'boolean' }); + }); + + it.each([ + { label: 'no credential fields', config: {} }, + { label: 'explicit opt-out', config: { useLocalGh: false } }, + { label: 'blank token', config: { token: ' ' } }, + { + label: 'cleared token with opt-out', + config: { token: '', useLocalGh: false }, + }, + ])('rejects a managed config with $label', async ({ config }) => { + const { plugin } = await import('./index.js'); + expect(plugin.management?.validateConfig?.(config)).toBe( + 'Channel requires a token or local GitHub CLI authentication (useLocalGh).', + ); + }); + + it.each([ + { label: 'literal token', config: { token: 'ghp_token' } }, + { + label: 'environment reference token', + config: { token: '$GITHUB_TOKEN' }, + }, + { label: 'local gh opt-in', config: { useLocalGh: true } }, + { + label: 'token and local gh opt-in', + config: { token: 'ghp_token', useLocalGh: true }, + }, + ])('accepts a managed config with $label', async ({ config }) => { + const { plugin } = await import('./index.js'); + expect(plugin.management?.validateConfig?.(config)).toBeUndefined(); + }); }); describe('validateCursor', () => { diff --git a/packages/channels/github/src/GithubAdapter.ts b/packages/channels/github/src/GithubAdapter.ts index 890e8dea5dc..938b64059ce 100644 --- a/packages/channels/github/src/GithubAdapter.ts +++ b/packages/channels/github/src/GithubAdapter.ts @@ -1,4 +1,5 @@ import { createHash, randomUUID } from 'node:crypto'; +import { execFile } from 'node:child_process'; import { appendFileSync, chmodSync, @@ -31,6 +32,115 @@ import { testBotMention, stripBotMention } from './mention.js'; interface GithubConfig extends ChannelConfig { baseUrl?: string; reasonFilter?: unknown; + useLocalGh?: boolean; +} + +const GH_AUTH_TIMEOUT_MS = 10_000; +const GH_AUTH_MAX_BUFFER = 64 * 1024; +// Same allowlist as the sibling gh wrappers, plus a leading-dash rejection so +// the value cannot be parsed as a gh option when passed to `gh auth token`. +const GH_HOSTNAME_RE = /^[A-Za-z0-9.-]+$/; + +function ghHostname(channelName: string, baseUrl: string): string { + let url: URL; + try { + url = new URL(baseUrl); + } catch { + throw new Error( + `[Channel:${channelName}] baseUrl is not a valid URL: ${baseUrl}`, + ); + } + if (!url.hostname) { + throw new Error( + `[Channel:${channelName}] baseUrl is not a valid URL: ${baseUrl}`, + ); + } + if (url.protocol !== 'https:') { + throw new Error( + `[Channel:${channelName}] local GitHub CLI authentication requires an HTTPS baseUrl.`, + ); + } + const hostname = + url.hostname === 'api.github.com' ? 'github.com' : url.hostname; + if (hostname.startsWith('-') || !GH_HOSTNAME_RE.test(hostname)) { + throw new Error( + `[Channel:${channelName}] baseUrl hostname is invalid: ${hostname}`, + ); + } + return hostname; +} + +// Sibling gh subprocess wrappers: core/src/utils/github-prs.ts, cli/src/commands/review/lib/gh.ts +function resolveGhAuthToken( + channelName: string, + hostname: string, +): Promise { + const env = { ...process.env }; + delete env['GH_TOKEN']; + delete env['GITHUB_TOKEN']; + delete env['GH_ENTERPRISE_TOKEN']; + delete env['GITHUB_ENTERPRISE_TOKEN']; + return new Promise((resolve, reject) => { + execFile( + 'gh', + ['auth', 'token', '--hostname', hostname], + { + timeout: GH_AUTH_TIMEOUT_MS, + maxBuffer: GH_AUTH_MAX_BUFFER, + windowsHide: true, + encoding: 'utf8', + env, + }, + (error, stdout, stderr) => { + if (error) { + const code = (error as NodeJS.ErrnoException).code; + let message: string; + if (code === 'ENOENT') { + message = + 'GitHub CLI (gh) is not installed on the daemon host or is not on the daemon PATH.'; + } else if ((error as { killed?: unknown }).killed === true) { + // Node sets killed=true even when the timed-out child also exited + // with a numeric code, so this must precede the exit-code branch. + message = `GitHub CLI authentication lookup for ${hostname} timed out after ${GH_AUTH_TIMEOUT_MS / 1000} seconds.`; + } else if (typeof code === 'number') { + // Matches go-gh's ConfigDir precedence: GH_CONFIG_DIR, + // XDG_CONFIG_HOME, %AppData%\GitHub CLI (Windows only), HOME. + message = `No GitHub CLI authentication is available for ${hostname}. Run \`gh auth login --hostname ${hostname}\` on the daemon host. gh config dir: ${ + env['GH_CONFIG_DIR'] || + (env['XDG_CONFIG_HOME'] + ? `${env['XDG_CONFIG_HOME']}/gh` + : process.platform === 'win32' && env['APPDATA'] + ? `${env['APPDATA']}\\GitHub CLI` + : env['HOME'] + ? `${env['HOME']}/.config/gh` + : 'unknown') + }`; + } else { + message = `GitHub CLI authentication lookup for ${hostname} failed to execute.`; + } + const stderrHint = stderr ? sanitizeLogText(stderr, 256).trim() : ''; + reject( + new Error( + `[Channel:${channelName}] ${message}${ + stderrHint ? ` gh stderr: ${stderrHint}` : '' + }`, + ), + ); + return; + } + const token = stdout.trim(); + if (!token) { + reject( + new Error( + `[Channel:${channelName}] GitHub CLI returned an empty token for ${hostname}. Run \`gh auth login --hostname ${hostname}\` on the daemon host.`, + ), + ); + return; + } + resolve(token); + }, + ); + }); } const KNOWN_NOTIFICATION_REASONS = new Set([ @@ -431,11 +541,28 @@ export class GithubChannel extends PollingChannelBase { const cfg = this.config as GithubConfig; this.reasonFilter = normalizeReasonFilter(cfg, this.name); const baseUrl = cfg.baseUrl || 'https://api.github.com'; + const configuredToken = cfg.token?.trim() ?? ''; + if (cfg.useLocalGh !== undefined && typeof cfg.useLocalGh !== 'boolean') { + throw new Error(`[Channel:${this.name}] useLocalGh must be a boolean.`); + } + if (!configuredToken && cfg.useLocalGh !== true) { + throw new Error( + `[Channel:${this.name}] configure a GitHub token or enable local GitHub CLI authentication.`, + ); + } + let auth = configuredToken; + let credential = 'configured token'; + if (!configuredToken) { + const hostname = ghHostname(this.name, baseUrl); + auth = await resolveGhAuthToken(this.name, hostname); + credential = `local gh credential for ${hostname}`; + } + process.stderr.write(`[Channel:${this.name}] using ${credential}\n`); this.webOrigin = baseUrl .replace(/\/api\/v3\/?$/, '') .replace(/^https:\/\/api\.github\.com/, 'https://github.com'); this.octokit = new Octokit({ - auth: cfg.token, + auth, baseUrl, ...(this.proxy ? { request: { agent: new HttpsProxyAgent(this.proxy) } } @@ -444,6 +571,9 @@ export class GithubChannel extends PollingChannelBase { try { const { data } = await this.octokit.rest.users.getAuthenticated(); this.botUsername = data.login; + process.stderr.write( + `[Channel:${this.name}] authenticated as "${sanitizeLogText(data.login, 64)}"\n`, + ); } catch (err) { throw new Error( `[Channel:${this.name}] failed to resolve bot identity: ${err}`, @@ -464,7 +594,7 @@ export class GithubChannel extends PollingChannelBase { ) { if (allowed.every((user) => user === botUsername)) { throw new Error( - `[Channel:${this.name}] GitHub allowlist only contains the authenticated GitHub account "${this.botUsername}", which cannot trigger this channel because self-authored comments are ignored. Use a separate bot-owned PAT and allowlist the operator account.`, + `[Channel:${this.name}] GitHub allowlist only contains the authenticated GitHub account "${this.botUsername}", which cannot trigger this channel because self-authored comments are ignored. Use a separate bot account (or a separate bot-owned PAT) and allowlist the operator account.`, ); } process.stderr.write( diff --git a/packages/channels/github/src/index.ts b/packages/channels/github/src/index.ts index 9e613000dd2..62c0f043301 100644 --- a/packages/channels/github/src/index.ts +++ b/packages/channels/github/src/index.ts @@ -6,7 +6,6 @@ export { GithubChannel }; export const plugin: ChannelPlugin = { channelType: 'github', displayName: 'GitHub', - requiredConfigFields: ['token'], envResolvableConfigFields: ['baseUrl'], defaultSessionScope: 'chat_thread', management: { @@ -15,9 +14,16 @@ export const plugin: ChannelPlugin = { key: 'token', label: 'Personal Access Token', kind: 'secret', - required: true, envResolvable: true, - description: 'Classic PAT with "notifications" scope', + description: + 'Optional classic PAT with "notifications" scope. Overrides local gh authentication', + }, + { + key: 'useLocalGh', + label: 'Use Local GitHub CLI Authentication', + kind: 'boolean', + description: + 'Reuse the daemon host GitHub CLI login when no token is configured', }, { key: 'baseUrl', @@ -90,6 +96,14 @@ export const plugin: ChannelPlugin = { ], }, ], + validateConfig: (config) => { + const token = + typeof config['token'] === 'string' ? config['token'].trim() : ''; + if (!token && config['useLocalGh'] !== true) { + return 'Channel requires a token or local GitHub CLI authentication (useLocalGh).'; + } + return undefined; + }, }, createChannel: (name, config, bridge, options) => new GithubChannel(name, config, bridge, options), diff --git a/packages/cli/src/commands/channel/channel-registry.test.ts b/packages/cli/src/commands/channel/channel-registry.test.ts index 5b68d249a8e..cf61b763d1e 100644 --- a/packages/cli/src/commands/channel/channel-registry.test.ts +++ b/packages/cli/src/commands/channel/channel-registry.test.ts @@ -28,13 +28,6 @@ describe('channel registry', () => { ); for (const type of ['github', 'gitlab'] as const) { const fields = catalog.find((entry) => entry.type === type)?.fields; - expect(fields).toContainEqual( - expect.objectContaining({ - key: 'token', - kind: 'secret', - required: true, - }), - ); expect(fields).toContainEqual( expect.objectContaining({ key: 'groupPolicy', @@ -56,6 +49,33 @@ describe('channel registry', () => { }), ); } + expect( + catalog.find((entry) => entry.type === 'gitlab')?.fields, + ).toContainEqual( + expect.objectContaining({ + key: 'token', + kind: 'secret', + required: true, + }), + ); + const githubFields = catalog.find( + (entry) => entry.type === 'github', + )?.fields; + expect(githubFields).toContainEqual( + expect.objectContaining({ + key: 'token', + kind: 'secret', + }), + ); + expect( + githubFields?.find((field) => field.key === 'token'), + ).not.toHaveProperty('required'); + expect(githubFields).toContainEqual( + expect.objectContaining({ + key: 'useLocalGh', + kind: 'boolean', + }), + ); expect(JSON.stringify(catalog)).not.toContain('createChannel'); }); }); diff --git a/packages/cli/src/serve/channel-settings-store.test.ts b/packages/cli/src/serve/channel-settings-store.test.ts index 07ada91ee78..fb4822ae1c6 100644 --- a/packages/cli/src/serve/channel-settings-store.test.ts +++ b/packages/cli/src/serve/channel-settings-store.test.ts @@ -485,6 +485,80 @@ describe('WorkspaceChannelSettingsStore', () => { }); }); + it('rejects a github channel with neither token nor local gh authentication without writing', async () => { + const store = new WorkspaceChannelSettingsStore(workspace); + const before = fs.readFileSync(settingsPath, 'utf8'); + + await expect( + store.upsert('bot', { + expectedRevision: store.snapshot().revision, + config: { + type: 'github', + senderPolicy: 'allowlist', + groupPolicy: 'open', + }, + }), + ).rejects.toMatchObject({ + code: 'channel_settings_invalid_config', + message: expect.stringContaining('local GitHub CLI authentication'), + }); + + expect(fs.readFileSync(settingsPath, 'utf8')).toBe(before); + }); + + it('rejects clearing the github token without local gh authentication', async () => { + writeWorkspaceSettings(`{ + "$version": 4, + "channels": { "bot": { + "type": "github", + "token": "existing-token", + "senderPolicy": "allowlist", + "groupPolicy": "open" + } } +}\n`); + const store = new WorkspaceChannelSettingsStore(workspace); + const before = fs.readFileSync(settingsPath, 'utf8'); + + await expect( + store.upsert('bot', { + expectedRevision: store.snapshot().revision, + config: { + type: 'github', + senderPolicy: 'allowlist', + groupPolicy: 'open', + }, + secrets: { token: { operation: 'clear' } }, + }), + ).rejects.toMatchObject({ + code: 'channel_settings_invalid_config', + message: expect.stringContaining('local GitHub CLI authentication'), + }); + + expect(fs.readFileSync(settingsPath, 'utf8')).toBe(before); + }); + + it('accepts a github channel that enables local gh authentication without a token', async () => { + const store = new WorkspaceChannelSettingsStore(workspace); + + const next = await store.upsert('bot', { + expectedRevision: store.snapshot().revision, + config: { + type: 'github', + useLocalGh: true, + senderPolicy: 'allowlist', + groupPolicy: 'open', + allowedUsers: ['operator'], + }, + }); + + expect(next.channels['bot']).toMatchObject({ + type: 'github', + useLocalGh: true, + senderPolicy: 'allowlist', + groupPolicy: 'open', + }); + }); + it('rejects clearing an existing required secret without writing', async () => { writeWorkspaceSettings(`{ "$version": 4, diff --git a/packages/cli/src/serve/channel-settings-store.ts b/packages/cli/src/serve/channel-settings-store.ts index d4458fc302b..7118ebb6264 100644 --- a/packages/cli/src/serve/channel-settings-store.ts +++ b/packages/cli/src/serve/channel-settings-store.ts @@ -386,6 +386,10 @@ export class WorkspaceChannelSettingsStore { if (value !== undefined) nextConfig[key] = value; } assertManagedConfig(nextConfig, previous, plugin.management.fields); + const crossFieldError = plugin.management.validateConfig?.(nextConfig); + if (crossFieldError !== undefined) { + throw invalidConfig(crossFieldError); + } const channels = { ...current.channels, [name]: nextConfig }; const workspaceFile = loadSettings(this.workspaceCwd, { diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx b/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx index 07054c667c2..aaf82c5a6a0 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx @@ -43,6 +43,24 @@ const OPTIONAL_SECRET: DaemonChannelTypeDescriptor = { ), }; +const GITHUB_LOCAL_GH: DaemonChannelTypeDescriptor = { + type: 'github', + displayName: 'GitHub', + manageable: true, + fields: [ + { + key: 'token', + label: 'Personal Access Token', + kind: 'secret', + }, + { + key: 'useLocalGh', + label: 'Use Local GitHub CLI Authentication', + kind: 'boolean', + }, + ], +}; + const INSTANCE: DaemonChannelInstanceSnapshot = { name: 'release-bot', config: { @@ -256,6 +274,40 @@ describe('ChannelEditorDialog', () => { expect(document.body.textContent).toContain('configured-user'); }); + it('saves the local GitHub CLI opt-in when the switch is toggled', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + await renderDialog({ descriptor: GITHUB_LOCAL_GH, onSave }); + + const name = inputByLabel('Instance name'); + const toggle = document.querySelector( + 'button[role="switch"]', + ); + expect(name).not.toBeNull(); + expect(toggle).not.toBeNull(); + + await act(async () => { + setInputValue(name!, 'github-bot'); + toggle!.click(); + }); + + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + + expect(onSave).toHaveBeenCalledWith('github-bot', { + expectedRevision: 'revision-1', + config: { + type: 'github', + useLocalGh: true, + senderPolicy: 'pairing', + }, + secrets: { token: { operation: 'clear' } }, + }); + }); + it('does not show the allowlist alert when no users are configured', async () => { const pairingNoAllowlist: DaemonChannelInstanceSnapshot = { ...INSTANCE, diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx b/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx index 75326c5dfdf..3a9ce6eed5c 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx @@ -74,6 +74,7 @@ const FIELD_LABEL_KEYS: Record> = { }, github: { token: 'channels.editor.field.github.token', + useLocalGh: 'channels.editor.field.github.useLocalGh', baseUrl: 'channels.editor.field.github.baseUrl', groupPolicy: 'channels.editor.field.github.groupPolicy', senderPolicy: 'channels.editor.field.github.senderPolicy', @@ -224,6 +225,8 @@ export function ChannelEditorDialog({ code: ChannelEditorValidationCode, ) => { if (code === 'duplicate') return t('channels.editor.validation.duplicate'); + if (code === 'credential') + return t('channels.editor.validation.credential'); if (code === 'invalid') return t('channels.editor.validation.invalidName'); if (code === 'invalidOption') return t('channels.editor.validation.invalidOption'); diff --git a/packages/web-shell/client/components/channels/channel-editor-state.test.ts b/packages/web-shell/client/components/channels/channel-editor-state.test.ts index fc15e9540ff..2affc36c286 100644 --- a/packages/web-shell/client/components/channels/channel-editor-state.test.ts +++ b/packages/web-shell/client/components/channels/channel-editor-state.test.ts @@ -135,6 +135,19 @@ describe('Channel editor state', () => { }); }); + it('does not clear a required secret from a blank replacement', () => { + const draft = createChannelEditorDraft(DINGTALK); + draft.name = 'release-bot'; + draft.values.clientId = 'ding-client-id'; + draft.secrets.clientSecret = { operation: 'replace', value: ' ' }; + + expect( + buildChannelUpsertRequest(DINGTALK, draft, 'revision-5').secrets, + ).toEqual({ + clientSecret: { operation: 'replace', value: ' ' }, + }); + }); + it('requires a unique name, required fields, a replacement secret, and an access policy', () => { const draft = createChannelEditorDraft(DINGTALK); draft.name = 'existing'; @@ -181,7 +194,11 @@ const GITHUB: DaemonChannelTypeDescriptor = { key: 'token', label: 'Personal Access Token', kind: 'secret', - required: true, + }, + { + key: 'useLocalGh', + label: 'Use Local GitHub CLI Authentication', + kind: 'boolean', }, { key: 'groupPolicy', @@ -230,7 +247,7 @@ describe('Descriptor-driven senderPolicy', () => { senderPolicy: 'pairing', allowedUsers: ['alice', 'bob'], }, - secrets: { token: { present: true, source: 'stored' } }, + secrets: { token: { present: true, source: 'literal' } }, startsWithServe: false, runtime: { state: 'stopped' }, }; @@ -244,7 +261,7 @@ describe('Descriptor-driven senderPolicy', () => { const instance: DaemonChannelInstanceSnapshot = { name: 'legacy-bot', config: { type: 'github' }, - secrets: { token: { present: true, source: 'stored' } }, + secrets: { token: { present: true, source: 'literal' } }, startsWithServe: false, runtime: { state: 'stopped' }, }; @@ -262,12 +279,112 @@ describe('Descriptor-driven senderPolicy', () => { const request = buildChannelUpsertRequest(GITHUB, draft, 'rev-1'); expect(request.config).toEqual({ type: 'github', + useLocalGh: false, groupPolicy: 'open', senderPolicy: 'allowlist', allowedUsers: ['alice', 'bob'], }); }); + it('requires a token or explicit local gh authentication', () => { + const draft = createChannelEditorDraft(GITHUB); + draft.name = 'my-bot'; + + expect(validateChannelEditorDraft(GITHUB, draft, [])).toEqual({ + token: 'credential', + }); + }); + + it('allows a new GitHub channel to opt into local gh authentication', () => { + const draft = createChannelEditorDraft(GITHUB); + draft.name = 'my-bot'; + draft.values.useLocalGh = true; + draft.secrets.token = { operation: 'replace', value: ' ' }; + + expect(validateChannelEditorDraft(GITHUB, draft, [])).toEqual({}); + const request = buildChannelUpsertRequest(GITHUB, draft, 'rev-1'); + expect(request.config).toMatchObject({ useLocalGh: true }); + expect(request.secrets).toEqual({ token: { operation: 'clear' } }); + }); + + it('round-trips useLocalGh from an existing channel draft', () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'my-bot', + config: { + type: 'github', + useLocalGh: true, + groupPolicy: 'open', + senderPolicy: 'allowlist', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + const draft = createChannelEditorDraft(GITHUB, instance); + expect(draft.values.useLocalGh).toBe(true); + const request = buildChannelUpsertRequest(GITHUB, draft, 'rev-1', instance); + expect(request.config).toMatchObject({ useLocalGh: true }); + }); + + it('clears an existing optional secret from a blank replacement', () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'my-bot', + config: { + type: 'github', + useLocalGh: true, + groupPolicy: 'open', + senderPolicy: 'allowlist', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + const draft = createChannelEditorDraft(GITHUB, instance); + draft.secrets.token = { operation: 'replace', value: ' ' }; + + expect( + buildChannelUpsertRequest(GITHUB, draft, 'rev-2', instance).secrets, + ).toEqual({ token: { operation: 'clear' } }); + }); + + it('replaces an existing optional secret with a non-blank value', () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'my-bot', + config: { + type: 'github', + useLocalGh: true, + groupPolicy: 'open', + senderPolicy: 'allowlist', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + const draft = createChannelEditorDraft(GITHUB, instance); + draft.secrets.token = { operation: 'replace', value: 'ghp_new' }; + + expect( + buildChannelUpsertRequest(GITHUB, draft, 'rev-6', instance).secrets, + ).toEqual({ token: { operation: 'replace', value: 'ghp_new' } }); + }); + + it('keeps an unchanged stored token valid while editing an existing channel', () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'my-bot', + config: { + type: 'github', + groupPolicy: 'open', + senderPolicy: 'allowlist', + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + const draft = createChannelEditorDraft(GITHUB, instance); + + expect(validateChannelEditorDraft(GITHUB, draft, [])).toEqual({}); + }); + it('skips senderPolicy validation when descriptor declares it', () => { const draft = createChannelEditorDraft(GITHUB); draft.name = 'my-bot'; diff --git a/packages/web-shell/client/components/channels/channel-editor-state.ts b/packages/web-shell/client/components/channels/channel-editor-state.ts index 70c9b484f2c..db84b618dca 100644 --- a/packages/web-shell/client/components/channels/channel-editor-state.ts +++ b/packages/web-shell/client/components/channels/channel-editor-state.ts @@ -28,6 +28,7 @@ export interface ChannelEditorDraft { export type ChannelEditorValidationCode = | 'required' + | 'credential' | 'duplicate' | 'invalid' | 'invalidOption' @@ -175,6 +176,13 @@ export function validateChannelEditorDraft( } } } + if (descriptor.type === 'github') { + const tokenField = descriptor.fields.find((f) => f.key === 'token'); + const hasToken = tokenField ? !isMissingField(tokenField, draft) : false; + if (!hasToken && draft.values['useLocalGh'] !== true) { + errors['token'] = 'credential'; + } + } if (!draft.senderPolicy && !hasDescriptorSenderPolicy(descriptor)) { errors['senderPolicy'] = 'policy'; } @@ -243,7 +251,9 @@ export function buildChannelUpsertRequest( const secret = draft.secrets[field.key] ?? { operation: 'preserve' }; secrets[field.key] = secret.operation === 'replace' - ? { operation: 'replace', value: secret.value ?? '' } + ? !field.required && !secret.value?.trim() + ? { operation: 'clear' } + : { operation: 'replace', value: secret.value ?? '' } : { operation: secret.operation }; continue; } diff --git a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts index 98f29e1e28f..70101070258 100644 --- a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts +++ b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts @@ -354,6 +354,145 @@ for (const theme of THEMES) { await captureScreenshot(page, `channel-editor-existing-${theme}`); }); + test(`GitHub channel editor`, async ({ page }, testInfo) => { + const scenario = createWebShellDaemonScenario({ + capabilities: { + features: [ + 'session_events', + 'permission_vote', + 'session_permission_vote', + 'session_scope_override', + 'session_source_metadata', + 'workspace_settings', + 'workspace_voice', + 'channel_management', + ], + }, + channelTypes: [ + { + type: 'github', + displayName: 'GitHub', + manageable: true, + fields: [ + { + key: 'token', + label: 'Personal Access Token', + kind: 'secret', + envResolvable: true, + }, + { + key: 'useLocalGh', + label: 'Use Local GitHub CLI Authentication', + kind: 'boolean', + }, + { + key: 'baseUrl', + label: 'Base URL', + kind: 'string', + envResolvable: true, + }, + { + key: 'groupPolicy', + label: 'Group Policy', + kind: 'enum', + required: true, + default: 'open', + options: [ + { value: 'open', label: 'Open' }, + { value: 'allowlist', label: 'Allowlist' }, + { value: 'disabled', label: 'Disabled' }, + ], + }, + { + key: 'senderPolicy', + label: 'Sender Policy', + kind: 'enum', + required: true, + options: [ + { value: 'allowlist', label: 'Allowlist' }, + { value: 'pairing', label: 'Pairing' }, + { value: 'open', label: 'Open' }, + ], + }, + { + key: 'allowedUsers', + label: 'Allowed Users', + kind: 'string-list', + }, + ], + }, + ], + channels: { revision: '1', instances: {} }, + }); + await page.addInitScript(() => { + window.sessionStorage.setItem('qwen-daemon-token', 'visual-token'); + }); + const daemon = await installScenario( + page, + scenario, + resolveBaseURL(testInfo), + ); + await gotoSession(page, scenario, daemon, theme); + + await page.getByRole('button', { name: 'Channels' }).click(); + await expect( + page.getByRole('heading', { name: 'Channels', level: 1 }), + ).toBeVisible(); + await page.getByRole('button', { name: 'Configure GitHub' }).click(); + await expect( + page.getByRole('heading', { name: 'Configure GitHub' }), + ).toBeVisible(); + await expect( + page.getByRole('switch', { + name: 'Use local GitHub CLI authentication', + }), + ).toBeVisible(); + await captureScreenshot(page, `github-channel-editor-${theme}`); + await page.getByLabel('Instance name').fill('github-bot'); + await page.getByRole('button', { name: 'Save' }).click(); + await expect( + page.getByText( + 'Enter a token or enable local GitHub CLI authentication.', + ), + ).toBeVisible(); + await captureScreenshot( + page, + `github-channel-editor-credential-${theme}`, + ); + await page + .getByRole('switch', { + name: 'Use local GitHub CLI authentication', + }) + .click(); + await page.getByRole('button', { name: 'Save' }).click(); + await expect( + page.getByText( + 'Enter a token or enable local GitHub CLI authentication.', + ), + ).toHaveCount(0); + await expect( + page.getByRole('heading', { name: 'Configure GitHub' }), + ).toHaveCount(0); + await expect + .poll(() => + daemon.requests.filter( + (request) => + request.method === 'PUT' && + request.path.endsWith('/channels/github-bot'), + ), + ) + .toEqual([ + expect.objectContaining({ + body: expect.objectContaining({ + config: expect.objectContaining({ + type: 'github', + useLocalGh: true, + }), + }), + }), + ]); + }); + test(`mermaid diagram`, async ({ page }, testInfo) => { const scenario = createWebShellDaemonScenario({ events: [ diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 063848f1112..e9e1f906693 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -2484,7 +2484,11 @@ const EN: Messages = { 'channels.editor.field.feishu.clientSecret': 'App Secret', 'channels.editor.field.github.token': 'Personal Access Token', 'channels.editor.field.github.token.description': - 'Classic PAT with "notifications" scope', + 'Optional classic PAT with "notifications" scope. Overrides local gh authentication', + 'channels.editor.field.github.useLocalGh': + 'Use local GitHub CLI authentication', + 'channels.editor.field.github.useLocalGh.description': + 'Explicitly reuse the daemon host’s account-wide gh login when no token is configured', 'channels.editor.field.github.baseUrl': 'API Base URL', 'channels.editor.field.github.baseUrl.description': 'GitHub Enterprise API root (e.g. https://ghe.example.com/api/v3). Leave empty for github.com', @@ -2597,6 +2601,8 @@ const EN: Messages = { 'Anyone who can reach the bot can start a conversation.', 'channels.editor.validation.required': (v) => `${v?.label ?? 'This field'} is required.`, + 'channels.editor.validation.credential': + 'Enter a token or enable local GitHub CLI authentication.', 'channels.editor.validation.duplicate': 'A Channel with this name already exists.', 'channels.editor.validation.invalidName': 'Choose a different instance name.', @@ -5038,7 +5044,10 @@ const ZH: Messages = { 'channels.editor.field.feishu.clientSecret': 'App Secret', 'channels.editor.field.github.token': '个人访问令牌', 'channels.editor.field.github.token.description': - '需要 "notifications" 权限的经典 PAT', + '可选。填写具有 "notifications" 权限的经典 PAT;优先于本地 gh 认证', + 'channels.editor.field.github.useLocalGh': '使用本地 GitHub CLI 认证', + 'channels.editor.field.github.useLocalGh.description': + '未配置令牌时,显式复用 daemon 主机上账户级的 gh 登录', 'channels.editor.field.github.baseUrl': 'API 基础 URL', 'channels.editor.field.github.baseUrl.description': 'GitHub Enterprise API 根地址(如 https://ghe.example.com/api/v3),github.com 留空', @@ -5146,6 +5155,8 @@ const ZH: Messages = { '所有能够访问机器人的用户均可直接开始对话。', 'channels.editor.validation.required': (v) => `${v?.label ?? '此字段'}为必填项。`, + 'channels.editor.validation.credential': + '请输入令牌,或开启本地 GitHub CLI 认证。', 'channels.editor.validation.duplicate': '已存在同名频道。', 'channels.editor.validation.invalidName': '请使用其他实例名称。', 'channels.editor.validation.invalidOption': '请移除不在允许列表中的值。',