-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(channels): support local gh authentication #8461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8055e6f
6911cb3
8807505
8e8e903
802c0f4
549bc39
55edd76
7deb8b4
a1acfba
90c4540
1e9d8df
fb29a25
a3868f1
d176b92
b313746
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <host>` 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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<Record<string, unknown>>, | ||
| ) => string | undefined; | ||
|
Comment on lines
+410
to
+412
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R9-2: This new plugin hook receives raw secret values in Failure scenario: a channel plugin echoes Suggested contract hardening (extend the JSDoc above this signature): /**
* ...
* `config` may contain raw secret values; the returned message is
* user-visible and must not include any config value.
*/中文说明[Suggestion] 这个新的插件 hook 会在 失败场景:某 channel 插件把 建议补充契约:在上方 JSDoc 中追加" — qwen3.8-max via Qwen Code /review (v0.21.5) |
||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] "Files affected" lists 6 files, but this PR changes 11 — and asymmetrically: it lists
channel-editor-state.test.tswithout its implementation file. Missing:channel-editor-state.ts(where the "token or useLocalGh" rule and the blank-replacement→clearbehavior live),ChannelEditorDialog.tsx,channel-registry.test.ts, andscreenshots.spec.ts. Sibling design docs use this section as the change footprint.Failure scenario: a maintainer using this doc to locate, modify, or revert the feature finds the tests but misses the editor-side enforcement implementation entirely.
中文说明
“Files affected” 只列出 6 个文件,而本 PR 改动了 11 个——且不对称:列出了
channel-editor-state.test.ts却没有其实现文件。遗漏:channel-editor-state.ts(“token 或 useLocalGh” 规则与空白替换→clear行为所在的文件)、ChannelEditorDialog.tsx、channel-registry.test.ts、screenshots.spec.ts。同类设计文档都把该节当作变更足迹使用。失败场景:维护者依据本文档定位、修改或回退该特性时,能找到测试,却完全漏掉编辑器侧的强制实现。
建议修复:补全缺失文件(见上方 suggestion 块)。
— qwen3.8-max via Qwen Code /review (v0.21.3)