fix(channels): exclude the channel's own API host and paths from URL shortening

Round-3 review findings on #16493, all closed in this commit:

- R1-1 (Critical, fix-induced): the host gate re-admitted the channel's own
  API host — with baseUrl set, a pasted REST URL shortened to `repos/octo`
  (api.github.com) or `api/v3` (GHES /api/v3), phantom repositories both
  validators accepted. Only the WEB host now qualifies: an api.-prefixed
  baseUrl host contributes only its stripped form, and paths under the
  baseUrl's own API prefix are excluded. Genuine GHES web URLs still
  shorten. Pinned with baseUrl actually passed through validation, for both
  REST shapes; re-admitting the raw API host reddens the test.
- R2-1: the baseUrl argument now has a call-site witness on both paths —
  a GHES draft validates clean and saves ['octo/repo'] through
  buildChannelUpsertRequest, mirrored in the parity table against the
  daemon authority. Dropping either argument reddens its half.
- Deferred notes (all three closed, none deferred onward): trailing
  sentence punctuation is stripped before matching (a pasted
  `…/octo/repo.` no longer becomes the phantom `octo/repo.`);
  `enterprises` joins the reserved set (snapshot updated); and a dialog
  case witnesses the parked-list round trip — hidden while Off, restored
  with its text on re-enable.

Mutation witnesses: re-admitting the raw API host, dropping the validator
baseUrl argument, and dropping the save-path baseUrl argument each fail
exactly their new test; all restored byte-identical.
This commit is contained in:
wenshao committed 2026-11-07 15:25:26 +08:00
1 parent 231d786eb3
commit d01ffd5108
6 files changed
+217 -53

No files matched your search

+13 -10
View File
@@ -108,16 +108,19 @@ Flat fields (not one object field) because the Web Shell editor cannot edit
- `channel-editor-state.ts`:
- `normalizeRepositoryFilterInput(value, baseUrl)` — splits on commas,
trims, lowercases, shortens GitHub web URLs
(`https://github.com/owner/repo`, with optional `.git`, trailing slash,
query, fragment, or `/pull/123`-style suffix) to `owner/repo`,
deduplicates, joins with `, `. Two gates decide whether a URL is
shortened at all. Structurally, only URLs whose host is the channel's
own GitHub host qualify: `github.com` plus the host derived from
`baseUrl` for GitHub Enterprise (an `api.` prefix is stripped); any
other host — `api.github.com`, `gist.github.com`, `gitlab.com`, an
arbitrary site — passes through unchanged for validation to reject,
which closes the "first two segments of any host" class. On a qualified
trims, strips trailing sentence punctuation a paste can carry, lowercases,
shortens GitHub web URLs (`https://github.com/owner/repo`, with optional
`.git`, trailing slash, query, fragment, or `/pull/123`-style suffix) to
`owner/repo`, deduplicates, joins with `, `. Two gates decide whether a
URL is shortened at all. Structurally, only URLs on the channel's own
GitHub WEB host qualify: `github.com` plus the web host derived from
`baseUrl` for GitHub Enterprise — an `api.`-prefixed baseUrl host
contributes only its stripped form, never the raw API host, and paths
under the baseUrl's own API prefix (e.g. `/api/v3/`) are excluded, so a
pasted REST URL cannot shorten to `repos/octo` or `api/v3`. Any other
host — `gist.github.com`, `gitlab.com`, an arbitrary site — passes
through unchanged for validation to reject, which closes the "first two
segments of any host" class. On a qualified
host, a URL whose first path segment is a reserved GitHub area (`orgs`,
`apps`, `settings`, `trending`, ...) still passes through unchanged so
validation rejects it instead of silently filtering a nonexistent
+11 -8
View File
@@ -96,14 +96,17 @@ Allowed repositories」下面。没有黑名单,而且访问策略的框架让
- `channel-editor-state.ts`:
- `normalizeRepositoryFilterInput(value, baseUrl)` —— 按逗号拆分、trim、
小写,把 GitHub web URL(`https://github.com/owner/repo`,可带 `.git`、
尾斜杠、query、fragment 或 `/pull/123` 式后缀)缩短为 `owner/repo`,
去重后用 `, ` 连接。是否缩短由两道门决定。结构上,只有主机属于频道
自己的 GitHub 主机才可缩短:`github.com` 加上由 `baseUrl` 派生的
GitHub Enterprise 主机(剥掉 `api.` 前缀);其他任何主机——
`api.github.com`、`gist.github.com`、`gitlab.com`、任意站点——原样
透传交给校验拒绝,从结构上关闭「任意主机的头两段路径」这一类隐患。
在合格主机上,首段路径是 GitHub 保留区域(`orgs`、`apps`、`settings`、
剥掉粘贴可能带上的句尾标点、小写,把 GitHub web URL
(`https://github.com/owner/repo`,可带 `.git`、尾斜杠、query、fragment
或 `/pull/123` 式后缀)缩短为 `owner/repo`,去重后用 `, ` 连接。是否
缩短由两道门决定。结构上,只有频道自己的 GitHub WEB 主机上的 URL 才
可缩短:`github.com` 加上由 `baseUrl` 派生的 GitHub Enterprise web
主机——带 `api.` 前缀的 baseUrl 主机只贡献去前缀后的形式,绝不加入
原始 API 主机;baseUrl 自身 API 前缀(如 `/api/v3/`)下的路径也被
排除,因此粘贴 REST URL 不会被缩短成 `repos/octo` 或 `api/v3`。其他
任何主机——`gist.github.com`、`gitlab.com`、任意站点——原样透传交给
校验拒绝,从结构上关闭「任意主机的头两段路径」这一类隐患。在合格
主机上,首段路径是 GitHub 保留区域(`orgs`、`apps`、`settings`、
`trending` 等)的 URL 仍原样透传,让校验拒绝它,而不是静默地去过滤
一个不存在的仓库。这份区域清单在构造上就是尽力而为——对 GitHub 顶层
路径做黑名单不可能穷尽,未来新增的区域在被补进清单之前会重新引入该
@@ -3998,6 +3998,37 @@ describe('GitHub repository event filter', () => {
expect(request.config['repoFilter']).toEqual(['octo/repo']);
});
it('hides the list while Off and restores it on re-enable', async () => {
await renderDialog({
descriptor: GITHUB_REPO_FILTER,
instance: {
...FILTER_INSTANCE,
config: {
...FILTER_INSTANCE.config,
repoFilterMode: 'blocklist',
repoFilter: ['octo/repo'],
},
},
});
expect(inputByLabel('Filtered Repositories')).toHaveProperty(
'value',
'octo/repo',
);
await selectOption('Repository Filter', 'Off — all repositories');
expect(inputByLabel('Filtered Repositories')).toBeNull();
await selectOption(
'Repository Filter',
'Blocklist — all except listed repositories',
);
expect(inputByLabel('Filtered Repositories')).toHaveProperty(
'value',
'octo/repo',
);
});
it('rejects entries that are not owner/repo after normalization', async () => {
const onSave = vi.fn().mockResolvedValue(undefined);
await renderDialog({
@@ -207,4 +207,34 @@ describe('repository filter rule parity (editor mirror vs daemon authority)', ()
expect(config['repoFilter']).toEqual(['octo/repo']);
expect(management.validateConfig?.(config)).toBeUndefined();
});
it('binds a GHES paste through the editor and the daemon authority', () => {
const instance: DaemonChannelInstanceSnapshot = {
name: 'parity',
config: {
type: 'github',
useLocalGh: true,
baseUrl: 'https://ghe.example.com/api/v3',
},
secrets: {},
startsWithServe: false,
runtime: { state: 'stopped' },
};
const draft = createChannelEditorDraft(descriptor, instance);
draft.name = 'parity';
draft.values['repoFilterMode'] = 'blocklist';
draft.values['repoFilter'] = 'https://ghe.example.com/octo/repo';
expect(validateChannelEditorDraft(descriptor, draft, [], instance)).toEqual(
{},
);
const { config } = buildChannelUpsertRequest(
descriptor,
draft,
'rev-1',
instance,
);
expect(config['repoFilter']).toEqual(['octo/repo']);
expect(management.validateConfig?.(config)).toBeUndefined();
});
});
@@ -2962,6 +2962,12 @@ describe('GitHub repository event filter', () => {
...GITHUB,
fields: [
...GITHUB.fields,
{
key: 'baseUrl',
label: 'Base URL',
kind: 'string',
envResolvable: true,
},
{
key: 'repoFilterMode',
label: 'Repository Filter',
@@ -3048,6 +3054,76 @@ describe('GitHub repository event filter', () => {
).toBe('octo/repo');
});
it('rejects REST-shaped URLs on the channel own API host', () => {
// The host gate must not re-admit the channel's own API host: with
// baseUrl set, a REST URL (the shape `gh api` prints) must pass through
// for validation to reject, not shorten to `repos/octo` or `api/v3`.
const cases = [
['https://api.github.com/repos/octo/repo', 'https://api.github.com'],
[
'https://ghe.example.com/api/v3/repos/octo/repo',
'https://ghe.example.com/api/v3',
],
];
for (const [url, baseUrl] of cases) {
expect(normalizeRepositoryFilterInput(url!, baseUrl)).toBe(
url!.toLowerCase(),
);
const draft = filterDraft('allowlist', url!);
draft.values.baseUrl = baseUrl!;
expect(validateChannelEditorDraft(REPO_FILTER, draft, [])).toEqual({
repoFilter: 'invalidRepoId',
});
}
});
it('strips trailing sentence punctuation from a pasted URL', () => {
expect(
normalizeRepositoryFilterInput(
'https://github.com/octo/repo., https://github.com/octo/repo)',
),
).toBe('octo/repo');
expect(
validateChannelEditorDraft(
REPO_FILTER,
filterDraft('blocklist', 'https://github.com/octo/repo.'),
[],
),
).toEqual({});
});
it('binds the GHES host through both the validator and the save path', () => {
// Dropping either baseUrl argument must fail here: the validator would
// reject the paste while the save path still shortens it (or vice
// versa) — the editor/daemon divergence the shared normalizer prevents.
const instance: DaemonChannelInstanceSnapshot = {
name: 'my-bot',
config: {
type: 'github',
useLocalGh: true,
baseUrl: 'https://ghe.example.com/api/v3',
},
secrets: {},
startsWithServe: false,
runtime: { state: 'stopped' },
};
const draft = filterDraft(
'blocklist',
'https://ghe.example.com/octo/repo',
instance,
);
expect(
validateChannelEditorDraft(REPO_FILTER, draft, [], instance),
).toEqual({});
const request = buildChannelUpsertRequest(
REPO_FILTER,
draft,
'rev-1',
instance,
);
expect(request.config['repoFilter']).toEqual(['octo/repo']);
});
it('does not shorten non-repository GitHub URLs', () => {
// Regression pins for the measured verify-round shapes: each of these
// once shortened to a plausible-but-nonexistent owner/repo that both
@@ -3094,6 +3170,7 @@ describe('GitHub repository event filter', () => {
'customer-stories',
'discussions',
'enterprise',
'enterprises',
'events',
'explore',
'features',
@@ -124,6 +124,7 @@ export const GITHUB_RESERVED_URL_PATHS = new Set([
'customer-stories',
'discussions',
'enterprise',
'enterprises',
'events',
'explore',
'features',
@@ -161,60 +162,79 @@ export const GITHUB_RESERVED_URL_PATHS = new Set([
/**
* The hosts whose two-segment paths may be repository URLs: github.com, plus
* the channel's own GitHub Enterprise host derived from baseUrl (the web host
* equals the API host there, and an `api.` prefix is stripped the same way
* githubEventWebUrl does). Any other host — api.github.com, gist.github.com,
* gitlab.com, an arbitrary site — passes through unchanged for validation to
* reject, which closes the "first two segments of any host" class structurally
* instead of through the area denylist.
* the channel's own GitHub Enterprise WEB host derived from baseUrl. Only the
* web host qualifies: an `api.`-prefixed baseUrl host denotes the deployment's
* API, so the stripped form is added and the raw API host is NOT (pasting a
* REST URL like https://api.github.com/repos/octo/repo must not shorten to
* `repos/octo`). The baseUrl's own API path prefix (e.g. /api/v3) is returned
* alongside so REST-shaped paths on a shared web+API host are excluded too.
* Any other host — gist.github.com, gitlab.com, an arbitrary site — passes
* through unchanged for validation to reject.
*/
function repositoryUrlHosts(baseUrl: string): Set<string> {
function repositoryUrlContext(baseUrl: string): {
hosts: Set<string>;
apiPath: string;
} {
const hosts = new Set(['github.com', 'www.github.com']);
let apiPath = '';
const trimmed = baseUrl.trim();
if (trimmed) {
try {
const host = new URL(trimmed).hostname.toLowerCase();
hosts.add(host);
if (host.startsWith('api.')) hosts.add(host.slice(4));
const url = new URL(trimmed);
const host = url.hostname.toLowerCase();
hosts.add(host.startsWith('api.') ? host.slice(4) : host);
const path = url.pathname.replace(/\/+$/, '').toLowerCase();
if (path) apiPath = path;
} catch {
// A malformed baseUrl is its own field's validation error; only
// github.com remains shortenable here.
}
}
return hosts;
return { hosts, apiPath };
}
/**
* Canonicalize the repository filter input: lowercase, deduplicate, and
* shorten pasted GitHub web URLs (repository, PR, or issue pages, with an
* optional `.git`, trailing path, query, or fragment) to `owner/repo` — only
* when the URL's host is the channel's own GitHub host. Entries that are not
* repository URLs on that host pass through unchanged so validation can name
* them. Shared by the validator and the save path so the two cannot disagree.
* Canonicalize the repository filter input: lowercase, deduplicate, strip
* trailing sentence punctuation a paste can carry (`.`, `)`, …), and shorten
* GitHub web URLs (repository, PR, or issue pages, with an optional `.git`,
* trailing path, query, or fragment) to `owner/repo` — only when the URL's
* host is the channel's own GitHub web host and its path is not the baseUrl's
* API prefix. Entries that are not repository URLs on that host pass through
* unchanged so validation can name them. Shared by the validator and the save
* path so the two cannot disagree.
*/
export function normalizeRepositoryFilterInput(
value: string,
baseUrl = '',
): string {
const hosts = repositoryUrlHosts(baseUrl);
const entries = toList(value).map((entry) => {
const match = GITHUB_REPOSITORY_URL_RE.exec(entry);
let host = '';
if (match) {
try {
host = new URL(entry).hostname.toLowerCase();
} catch {
host = '';
const { hosts, apiPath } = repositoryUrlContext(baseUrl);
const entries = toList(value)
.map((raw) => raw.replace(/[.,;:!?)\]]+$/, ''))
.filter(Boolean)
.map((entry) => {
const match = GITHUB_REPOSITORY_URL_RE.exec(entry);
let url: URL | undefined;
if (match) {
try {
url = new URL(entry);
} catch {
url = undefined;
}
}
}
const shortened =
match &&
hosts.has(host) &&
!GITHUB_RESERVED_URL_PATHS.has(match[1].toLowerCase())
? `${match[1]}/${match[2]}`
: entry;
return shortened.toLowerCase();
});
const onApiPath =
url !== undefined &&
apiPath !== '' &&
url.pathname.toLowerCase().startsWith(`${apiPath}/`);
const shortened =
match &&
url !== undefined &&
hosts.has(url.hostname.toLowerCase()) &&
!onApiPath &&
!GITHUB_RESERVED_URL_PATHS.has(match[1].toLowerCase())
? `${match[1]}/${match[2]}`
: entry;
return shortened.toLowerCase();
});
return [...new Set(entries)].join(', ');
}