Uh oh!
There was an error while loading. Please reload this page.
ci: bug-bash workflow — record CLI TUI to S3 on every PR - #2131
Conversation
Adds .github/workflows/bug-bash.yml + .github/harness/bug-bash/record.mjs. Runs on pull_request (same-repo) + workflow_dispatch: builds the CLI, records the TUI via private-tui-harness, uploads the MP4 to S3 keyed by repo/pr-number. Assumes the shared E2E role via the devx-devtools fetch-secrets action, per the Moab reusable-workflow convention.
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
A few things need attention before this can land safely.
1. Unpinned clone of a personal GitHub repo executed with AWS credentials — .github/workflows/bug-bash.yml lines 83–86
git clone --depth 1 https://github.com/jariy17/private-tui-harness.git "$RUNNER_TEMP/tui-harness"(cd "$RUNNER_TEMP/tui-harness" && npm ci && npm run build)This clones the default branch of a personal user repo (jariy17/private-tui-harness) on every run, then runs npm ci (executes install scripts) and node dist/index.jsafter the E2E AWS role has been assumed. Whoever controls that personal account can push code that exfiltrates the shared E2E_AWS_ROLE_ARN credentials or writes anywhere the role can reach. The header comment in the workflow specifically calls out pinning the composite action to a full SHA "per the guide" — the same rule needs to apply here.
Options:
- Move
private-tui-harnessunder theaws/org and pin the checkout to a full commit SHA. - Publish it as a private npm package fetched via CodeArtifact / a scoped token, and pin the version.
- At the very least, pin to a full commit SHA (
git -C … checkout <sha>) and add SHA verification, but org-owned is strongly preferred given this runs with production-adjacent AWS creds.
2. Bun is used but the repo has no bun.lockb — .github/workflows/bug-bash.yml lines 78–80
- uses: oven-sh/setup-bun@v2
- run: bun install --frozen-lockfile
- run: bun run buildThe repo ships package-lock.json and every other workflow uses npm ci / npm run build. bun install --frozen-lockfile requires a bun.lockb and will fail on main today. Please either switch this job to npm ci / npm run build for consistency with the rest of the workflows, or commit a bun lockfile and justify introducing a second package manager to CI.
3. Runner label appears not to be provisioned — .github/workflows/bug-bash.yml line 43
runs-on: codebuild-agentcore-e2e-${{ github.run_id }}-${{ github.run_attempt }}No other workflow in this repo (or in agentcore-l3-cdk-constructs) targets a codebuild-agentcore-… self-hosted runner — the existing E2E workflows all use ubuntu-latest. If this CodeBuild runner project hasn't actually been created, every run will queue forever. Please confirm the runner is provisioned, or switch to ubuntu-latest to match the existing E2E jobs.
4. Authorization on same-repo PRs — .github/workflows/bug-bash.yml lines 40–42
The gate only excludes forks. Compare with e2e-tests.yml, which additionally checks AUTHORIZED_USERS before assuming the E2E role. Because this workflow runs pull_request (not pull_request_target) it executes PR-head code (including a modifiable record.mjs and BUGBASH_* env vars) with the shared runtime role. That's the same trust model as E2E, so it should carry the same authorize gate — otherwise any collaborator with push access can drive arbitrary behavior under those credentials.
Minor (please address if easy)
record.mjsdefaultsBUGBASH_CMD=bunandBUGBASH_ARGS='run src/index.ts', which runs source rather than the artifact you just built withbun run build. Consider defaulting to the built CLI so the recording reflects what users actually get.- No telemetry hook is added, but this is CI-only tooling so that's fine.
Once #1–#3 are resolved (and #4 acknowledged one way or the other) this should be good to go.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## refactor #2131 +/- ##
=========================================
Coverage 97.24% 97.24% =========================================
Files 465 465 Lines 28417 28417 =========================================
Hits 27635 27635 Misses 782 782 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Claude Security Review: no high-confidence findings. (run) |
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
I ran through the diff independently and the four blocking issues I found are already covered in the prior automated review:
- Cloning an unpinned personal repo (
jariy17/private-tui-harness) after assuming AWS creds —.github/workflows/bug-bash.ymlL83–86. bun install --frozen-lockfilewith nobun.lockbin the repo — L78–80. Confirmed onlypackage-lock.jsonexists.runs-on: codebuild-agentcore-e2e-…self-hosted label that no other workflow in this repo (or inagentcore-l3-cdk-constructs) uses — L43.- Missing
AUTHORIZED_USERSgate on same-repo PRs; unlikee2e-tests.yml, this workflow runs onpull_request(PR-head code) with the shared runtime role and only excludes forks — L40–42.
A couple of small extras that aren't blockers but worth a look while you're in there:
record.mjsdefaultsBUGBASH_CMD=bun/BUGBASH_ARGS='run src/index.ts', which executes source rather than thebun run buildoutput. Consider pointing at the built artifact so the recording matches what ships. (Already mentioned in prior review.)- The final S3 upload step prints "check S3" to the step summary but not the bucket/key layout, so reviewers have to know the convention out-of-band. Non-blocking, but including the key (not a presigned URL) in the summary would make triage a lot easier.
Nothing new from me — once the four items above are addressed this should be safe to land.
Claude Security Review: no high-confidence findings. (run) |
Status:
This PR (
bug-bash.yml+record.mjs) — shelvedThe CI caller here assumes the shared 685197708687 E2E role and fetches secrets from 631957124172 — cross-account. That conflicts with the explore-only requirement, so it is not for merge; kept as reference. The runtime-bot path above supersedes it.
Open reviewer findings (only if this CI path is revived)
private-tui-harnessclone to a full commit SHA / org-own it (supply-chain).AUTHORIZED_USERSgate before assuming the AWS role (mirrore2e-tests.yml).