tiennm99/ccs
1
0
Fork 0
mirror of https://github.com/tiennm99/ccs.git synced 2026-07-16 06:16:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ci): reviewer loop 6 — REV12 compose image parsing, REV13 fork bypass, REV14 :full audit (#1278)

Browse Source 
* fix(ci): breaking-change-guard handles \${VAR:-default} compose image syntax (REV12)

The sed 's/:.*//' pattern truncated at the first colon in the
\${CCS_IMAGE:-ghcr.io/...} expression, yielding "\${CCS_IMAGE" as the
image name instead of the actual registry path. Any change to the default
image namespace was therefore undetectable.

Introduce extract_image_name() that first strips the \${VAR:-default}
wrapper with a sed -E expression, then strips only the trailing :tag
suffix using a pattern that preserves internal colons (e.g. registry:5000/
owner/repo). Applied to both OLD_RAW and NEW_RAW extraction paths.

* fix(ci): breaking-change-guard runs on ubuntu-latest to cover forked PRs (REV13)

The trusted-author gate (COLLABORATOR|MEMBER|OWNER) caused forked-PR
contributors to bypass the breaking-change check entirely. A forked
contributor could rename services.ccs or change the image namespace
without a feat!/fix! marker and the guard would never run.

This workflow is a documented exception to the self-hosted-first policy:
it performs ONLY pure YAML diff parsing (git show / awk / sed). No build,
install, or arbitrary PR-branch scripts are executed. The checkout uses
persist-credentials: false. There is no untrusted code execution, so
ubuntu-latest is safe and necessary for universal fork coverage.

Update self-hosted-runner-policy.test.ts to:
- Introduce GITHUB_HOSTED_RUNNER_EXCEPTIONS registry with required
  justification comments for each entry
- Skip exception workflows in the "keeps active workflows on local runners"
  and "gates pull-request workflows" assertions
- Add a new "documented exceptions use github-hosted runners" test that
  verifies each exception entry actually uses a GitHub-hosted runner
  (prevents stale entries accumulating without cleanup)
This commit is contained in:
Kai (Tam Nhu) Tran
2026-05-17 06:01:25 -04:00
committed by GitHub
parent 4e9c14b64a
commit e7ce699dc2
2 changed files with 61 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/breaking-change-guard.yml
+31 -7
View File
@@ -22,9 +22,17 @@ on:
jobs:
guard:
name: Verify breaking changes are intentional
if: >-
contains(fromJSON('["COLLABORATOR","MEMBER","OWNER"]'), github.event.pull_request.author_association)
runs-on: [self-hosted, linux, x64, cliproxy]
# Exception to self-hosted-first policy (documented in CLAUDE.md "Self-Hosted Runner Policy"):
# This workflow MUST cover ALL PRs including forks — a forked contributor can rename
# services.ccs or change the image namespace without a breaking-change marker, which
# silently breaks sibling-container setups for every user. Gating on trusted-author
# association would let forked PRs bypass the check entirely.
#
# Safety justification: this workflow does ONLY pure YAML diff parsing (git show / git diff
# + shell + awk). It checks out the code with persist-credentials: false and runs no build,
# install, or arbitrary scripts from the PR branch. There is no untrusted code execution,
# so running on a GitHub-hosted runner is safe and required for universal coverage.
runs-on: ubuntu-latest
steps:
- name: Checkout PR branch
@@ -54,10 +62,26 @@ jobs:
BREAKING=0
# 1. Image name change (repo path, not tag — tags change every release)
OLD_IMAGE=$(git show "${BASE}:docker/compose.yaml" \
| grep -m1 '^\s*image:' | sed 's/.*image:\s*//' | sed 's/:.*//' | tr -d ' ')
NEW_IMAGE=$(grep -m1 '^\s*image:' docker/compose.yaml \
| sed 's/.*image:\s*//' | sed 's/:.*//' | tr -d ' ')
#
# compose.yaml uses ${CCS_IMAGE:-ghcr.io/kaitranntt/ccs:latest}. A naive
# sed 's/:.*//' truncates at the FIRST colon, yielding "${CCS_IMAGE" instead
# of the actual image name. We strip the ${VAR:-default} wrapper first, then
# strip only the trailing :tag suffix (preserving internal colons such as
# those in registry:port/owner/repo).
extract_image_name() {
# $1 = raw image line content (everything after "image: ")
local raw="$1"
# Strip ${VAR:-default} wrapper if present
raw=$(printf '%s' "$raw" | sed -E 's/^\$\{[A-Za-z_][A-Za-z0-9_]*:-//; s/\}$//')
# Strip only the trailing :tag — preserve internal colons (e.g. registry:5000/owner/repo)
printf '%s' "$raw" | sed 's|:[^:/]*$||'
}
OLD_RAW=$(git show "${BASE}:docker/compose.yaml" \
| grep -m1 '^\s*image:' | sed 's/.*image:\s*//' | tr -d ' ')
NEW_RAW=$(grep -m1 '^\s*image:' docker/compose.yaml \
| sed 's/.*image:\s*//' | tr -d ' ')
OLD_IMAGE=$(extract_image_name "$OLD_RAW")
NEW_IMAGE=$(extract_image_name "$NEW_RAW")
if [[ "${OLD_IMAGE}" != "${NEW_IMAGE}" ]]; then
echo "[!] BREAKING: image name changed: '${OLD_IMAGE}' -> '${NEW_IMAGE}'"
BREAKING=1
tests/unit/scripts/github/self-hosted-runner-policy.test.ts
+30
View File
@@ -7,6 +7,17 @@ function workflowsDir() {
return path.resolve(import.meta.dir, '../../../../.github/workflows');
}
// Documented exceptions to the self-hosted-first policy (see CLAUDE.md "Self-Hosted Runner Policy").
// Each entry must include a justification comment explaining why GitHub-hosted runners
// are required for correctness (not just convenience).
const GITHUB_HOSTED_RUNNER_EXCEPTIONS: Record<string, string> = {
// Pure YAML diff parser — no untrusted code execution. Must cover ALL PRs including
// forks to prevent forked contributors from bypassing the breaking-change check.
// Gating on trusted-author association would silently allow contract-breaking changes
// from forks. No build, install, or arbitrary PR-branch scripts are run here.
'breaking-change-guard.yml': 'ubuntu-latest — fork-safe YAML diff check; no untrusted code execution',
};
describe('self-hosted runner policy', () => {
test('keeps active workflows on local runners', () => {
const hostedRunnerLabels = [
@@ -23,6 +34,9 @@ describe('self-hosted runner policy', () => {
expect(workflowFiles.length).toBeGreaterThan(0);
for (const file of workflowFiles) {
// Skip files with documented, justified exceptions to the self-hosted-first policy
if (GITHUB_HOSTED_RUNNER_EXCEPTIONS[file]) continue;
const workflow = fs.readFileSync(path.join(workflowsDir(), file), 'utf8');
for (const label of hostedRunnerLabels) {
@@ -35,6 +49,17 @@ describe('self-hosted runner policy', () => {
}
});
test('documented exceptions use github-hosted runners for justified safety reasons', () => {
// Verify each documented exception actually uses a GitHub-hosted runner
// (prevents stale exception entries that no longer reflect the workflow)
for (const [file, reason] of Object.entries(GITHUB_HOSTED_RUNNER_EXCEPTIONS)) {
const workflow = fs.readFileSync(path.join(workflowsDir(), file), 'utf8');
const hasGitHubHosted = ['ubuntu-latest', 'ubuntu-24.04', 'ubuntu-22.04', 'macos-latest', 'windows-latest']
.some((label) => workflow.includes(`runs-on: ${label}`));
expect(hasGitHubHosted, `${file} is in exceptions list (reason: ${reason}) but does not use a GitHub-hosted runner — remove the exception or restore the runner type`).toBe(true);
}
});
test('gates pull-request self-hosted worker deploys to trusted authors', () => {
const workflow = fs.readFileSync(path.join(workflowsDir(), 'deploy-ccs-worker.yml'), 'utf8');
@@ -60,6 +85,11 @@ describe('self-hosted runner policy', () => {
);
}
// Documented exceptions run on GitHub-hosted runners for justified safety reasons
// (e.g. must cover forked PRs, no untrusted code execution). These workflows do not
// use self-hosted runners for their PR jobs, so the trusted-author gate does not apply.
if (GITHUB_HOSTED_RUNNER_EXCEPTIONS[file]) continue;
if (
workflow.includes('pull_request:') &&
workflow.includes('self-hosted') &&