Commit graph

3 commits

Author SHA1 Message Date
Shaojin Wen
06cc41ee3f
ci: route trusted-author fork PRs and no-checkout jobs to the ECS pool (#8502)
Some checks failed
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 1/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 2/3 (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none - shard 3/3 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 1/2 (push) Waiting to run
E2E Tests / E2E Test - macOS - shard 2/2 (push) Waiting to run
E2E Tests / channel-plugin E2E (nightly) (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
npm cache producer / Save npm cache (push) Has been cancelled
* ci: route trusted-author fork PRs and no-checkout jobs to the ECS pool

Fork PRs whose author has write access (OWNER/MEMBER/COLLABORATOR association) now run Linux CI on the self-hosted ECS pool instead of the saturated GitHub-hosted quota, and bot workflows that check out no code move to ECS unconditionally. Everything stays gated on the MAINTAINER_ECS_RUNNER_DISABLED kill-switch.

* ci: address review — real write-permission routing, watchdog independence, timeouts

Route the triage agent on the collaborator-permission API result computed by authorize instead of the coarse author_association, which admits org members and read-only collaborators; the two permission-gate jobs revert to the same-repo guard. Keep the fleet watchdog and the CI-failure reporter hosted so they stay independent of the pool they watch. Add missing timeouts, wipe serve-ab's reused workspace, and pin the routing logic with drift and negative-case tests.

---------

Co-authored-by: 易良 <1204183885@qq.com>
2026-08-04 03:48:24 +00:00
Shaojin Wen
f4cd6e1d8b
fix(ci): gate the attachment guard before it allocates a runner (#8095)
Measured on a congested pool: 88 active jobs, 72 hosted and 15
self-hosted. The self-hosted 15 were all running with zero queued; the
hosted 72 were contending, and 20 of them were the SAME job —
`remove-suspicious-attachments`, all queued, none running.

Its real cost is not the work. Recent completed runs:

    queue=629s run=5s      queue=568s run=2s
    queue=518s run=2s      queue=340s run=3s

Two to five seconds of API calls behind up to ten minutes of queueing.

And almost none of it needed to happen. The trust check lived INSIDE the
github-script, so a runner was queued, allocated and started before the
job could decide it had nothing to do. Over the 200 most recent comments
on this repo: 184 from trusted associations, 9 from bots, 7 actually
needing a scan. 96.5% of these runs existed to print "Trusted author;
skipping".

Two changes:

- Hoist the association and bot checks into the job `if:`. GitHub
  evaluates `if:` BEFORE allocating a runner, so a trusted comment now
  costs nothing. The script keeps its own copies: the gate is an
  optimisation, not the control, and the two must be able to disagree
  without becoming unsafe. Every ambiguity therefore resolves toward
  RUNNING the scan — an unrecognised payload yields an empty
  association, which is not in the trusted list, so the job runs.

- Add a per-comment concurrency group with cancel-in-progress. The
  workflow listens on `edited` as well as `created`, and the bot PATCHes
  its own comments constantly, so repeated edits of one comment stacked.
  The scan reads the comment's CURRENT body, so a queued earlier scan is
  already stale and cancelling it loses nothing. (Contrast the verify
  lane, where cancel-in-progress is deliberately false because a
  cancelled run destroys evidence.) The key falls back to run_id so an
  unexpected payload gets its own group instead of serialising every
  scan into one.

Deliberately NOT moved to the self-hosted pool, though it would fit
technically (no checkout, no PR code, API calls only): the 20 stacked
jobs were duplicates, so relocating them just fills the ECS pool
instead — and that pool is what /verify and /triage depend on. It also
holds issues:write while processing untrusted comment bodies, which
belongs on ephemeral hardware rather than reused machines.

The `if:` semantics are verified against all payload shapes — 12 cases
covering both `comment.*` and `review.*` associations, bots, and
missing/empty payloads, each asserting which direction it resolves.
CONTRIBUTOR is deliberately NOT trusted: a merged PR does not make
someone's links safe.

Mutation-verified 6/6: dropping the review payload path, dropping the
bot check, adding CONTRIBUTOR to the trusted list, turning off
cancel-in-progress, collapsing the group to a global key, and inverting
the gate so untrusted comments are the ones skipped — each turns a test
red. The last is the one that matters; it is the only mutation here that
would be a security regression rather than a cost regression.

148/148 tests across both suites; actionlint exit 0; prettier and eslint
clean.

Co-authored-by: wenshao <wenshao@example.com>
2026-07-30 06:32:55 +00:00
易良
3bf2d45403
ci: add suspicious comment attachment guard (#6599)
* ci: add suspicious comment attachment guard

Resolves #6597

* ci: reduce attachment guard false positives

* ci: harden comment attachment guard

* ci: tighten attachment extension matching

* ci: avoid false attachment removal summary

* ci: avoid markdown link attachment false positives

* ci: avoid country-code attachment false positives

* ci: harden attachment URL parsing

* ci: reduce attachment guard false positives

Updates #6597

* ci: harden malformed attachment URLs

Updates #6597

* ci: reduce attachment guard overmatching

Updates #6597

* ci: scan attachment path segments

Updates #6597

* ci: cover review attachment summaries

* ci: harden attachment link detection

* fix: address critical bypass vectors in comment attachment guard

- Remove break in decodeTarget catch block so malformed percent sequences
  (e.g., %ZZ) don't prematurely exit the decode loop, allowing
  double-encoded extensions to be fully detected
- Strip zero-width characters (U+200B-U+200D, U+FEFF, U+00AD, U+2060,
  U+180E) before NFKC normalization to prevent invisible-character
  evasion of extension matching
- Add protocol-relative URL (//) support to linkPattern and
  highRiskTarget, normalizing to https: before parsing
- Add tests for all three bypass vectors (39 total, all passing)

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-10 09:40:54 +00:00