revert: revert security fix PRs #31 and #33 to unblock deployments [E-1815] - #35
Merged
Conversation
The mobbdev CLI now prefixes its output with status messages like "[WebSocket Mode] Using WebSocket subscription..." before the URL. Extract just the https:// URL using grep. Ref: E-1815
|
Kirill89
approved these changes
Apr 8, 2026
antonychiu2
added a commit
that referenced
this pull request
Aug 14, 2026
* feat(review): make report-file optional for scan-and-fix mode Allow omitting report-file in the review (gh-fixer) action so the Mobb CLI can run its own internal SAST scan (opengrep) instead of requiring an external SARIF/JSON report. -f is appended only when report-file is provided. * feat(review): make scanner optional for scan-and-fix mode Allow omitting scanner alongside report-file. --scanner is appended only when the input is non-empty, so scan-and-fix mode (no external report) no longer requires a scanner declaration. * feat(review): migrate to @mobb.ai/cli and fail fast on missing required args Bugsy now ships as @mobb.ai/cli — a prebuilt standalone binary, same code and version stream as mobbdev, but no longer tied to the user's Node version. The CLI's review command hard-requires -f, --scanner and --ch (verified against 1.4.57: omitting them yields "Missing required arguments: f, scanner, ch, commit-hash"). Combined with the previous `eval ... || true`, that failure was silent: the step went green, fix-report-url was empty, the status step was skipped by its startsWith guard, and the user saw a passing check with no fix report. - switch to npx --yes @mobb.ai/cli@latest, bump setup-node v3.6.0 -> v4 - validate report-file/scanner/commit-hash/pr-number up front with an actionable ::error:: message; GitHub does not reliably enforce `required:` for composite actions - validate scanner against the CLI's own --scanner choices - add fail-on-error input (default false, preserving the historical non-blocking behaviour) and always emit an ::error:: annotation on non-zero exit instead of swallowing it - echo the CLI's stdout, which command substitution was hiding entirely - add src-path (-p) and polling inputs - quote $OUT in the tr pipeline so output containing * cannot glob - fix "GitaHub Token" typo organization-id is deliberately not wired here: the CLI rejects it on review with "Unknown argument: org". Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * feat: migrate root action to @mobb.ai/cli and expose new CLI flags Switch to npx --yes @mobb.ai/cli@latest (standalone binary, same 1.4.57 code as mobbdev) and bump setup-node v3.6.0 -> v4. New inputs wired to flags the current CLI supports: - polling -> --polling (for runners behind proxies/firewalls blocking WSS) - create-one-pr-> --create-one-pr, appended only alongside --auto-pr, with a fail-fast check when set without auto-pr - src-path -> -p, for monorepos - scan-only -> --scan-only Also: - guard the status step with startsWith(..., 'https://'), matching the review action, so it no longer posts a status with an empty target_url when URL extraction fails - echo the CLI's stdout, which command substitution was hiding entirely - quote $OUT in the tr pipeline so output containing * cannot glob - fix "GitaHub Token" typo Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * feat: pass --pr-id for auto-pr, not only commit-directly --pr-id was previously appended only inside the commit-directly branch, so an --auto-pr run in a pull request context never told Mobb which PR the fixes belong to. Pass it whenever a PR context exists and either auto-pr or commit-directly is enabled. Kept as an isolated commit so it can be dropped if the coupling to commit-directly was intentional. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * test: add lint, CLI contract, and analyze-mode test workflows This repo had no real CI signal: both existing workflows always passed report-file, so scan-and-fix, diff-aware, auto-pr and commit-directly were never exercised, and nothing checked that the flags the actions emit still exist in the CLI. Review-mode tests live in their own files (added separately) so a review regression shows up as its own red check. lint.yml - actionlint over the workflows, with -shellcheck= since shellcheck floods on the deliberate eval pattern (see reverted PRs #31/#33/#35) - .github/scripts/validate-actions.py for the composite action.yml files, which actionlint cannot lint: it parses them as workflows and fails on the missing on/jobs sections. The script bash -n's every run block, checks that every inputs.<name> reference is declared and every declared input used, and checks the README examples only pass real inputs -- which immediately caught the documented-but-nonexistent auto-commit input. cli-smoke.yml (no secrets, runs on fork PRs, no Mobb quota) - binary-ubuntu: required leg proving the platform binary resolves and runs. @mobb.ai/cli is a ~3 KB launcher that resolves a binary from optionalDependencies rather than downloading one, so it hard-fails on musl/Alpine, win-arm64, and wherever optional deps are skipped. - binary-other-os: macOS/Windows, continue-on-error, informational only. - analyze-contract: asserts analyze still documents every flag action.yml passes, and that -f stays optional so scan-and-fix keeps working. - review-contract: asserts review still documents the flags review/action.yml passes and still requires f, scanner and ch. This is the standing alarm for the mismatch this branch fixes. test-analyze-args.yml - asserts the exact command the root action builds for each input combination via the MOBB_ACTION_DRY_RUN seam, including the negative case of create-one-pr without auto-pr. No API calls. test-analyze-e2e.yml - real runs over the repo's vulnerable fixtures: scan-and-fix, diff-aware, and a manual-only auto-pr leg. Asserts fix-report-url is a real URL. main.yml - checkout/setup-node v3 -> v4, skip on fork PRs instead of failing red on missing secrets, and assert fix-report-url starts with https:// Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * test: add dedicated review-mode test workflows Review mode gets its own workflows so that when it breaks the failing check says "review" rather than hiding inside an analyze-mode failure. test-review-e2e.yml - CodeQL -> ./review with fail-on-error: true, so a CLI failure propagates instead of producing the green-check-with-no-fix-report that the previous `eval ... || true` allowed. Asserts fix-report-url is a real URL. - daily schedule, so a CLI-side regression surfaces with no open PR - a secret-free negative job asserting that omitting report-file fails the step and produces no fix-report-url test-review-args.yml - asserts the command review/action.yml builds (-f, --scanner, --ch, --pr, --github-token) plus pass-through of mobb-project-name, src-path and polling - negative cases: no report-file, no scanner, unsupported scanner, and no pull_request context review.yml - kept as the reference example users copy; bumped checkout v3 -> v4 and codeql-action v2 -> v3 (both flagged by actionlint as too old to run), added the fork-PR guard and the permissions CodeQL needs Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * docs: document the new CLI, new inputs, and the review action's requirements - fix the scan-and-fix example: it passed `auto-commit: true`, which no action has ever declared, so it was silently dropped. The real input is `commit-directly`. The new metadata validator now catches this class of bug. - add a Requirements section naming @mobb.ai/cli and documenting the Alpine/musl, win-arm64 and --no-optional caveat, with `npx mobbdev@latest` as the fallback for those runners - document the new inputs: create-one-pr, src-path, polling, fail-on-error - add a Review action section: the README previously framed the repo as having two modes without mentioning that review mode requires report-file and scanner and only runs on pull_request events. The CLI hard-requires all of them, so this is now stated plainly, along with a worked example and a note that organization-id is rejected on review. - add a Versioning section noting the v1 tag currently lags the v1.x tags - drop the specific scanner-engine name from the scan-and-fix description; the internal SAST implementation is in flux scan-only is deliberately left undocumented while Mobb SAST settles. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * Revert "feat: pass --pr-id for auto-pr, not only commit-directly" This reverts commit b85c453. * fix: enforce the CLI's auto-pr flag constraints Testing the generated commands against @mobb.ai/cli 1.4.57 surfaced two constraints the action did not respect: 1. "--create-one-pr and --commit-directly cannot be provided at the same time". The action would happily pass both. Now rejected up front with an explanation of why they conflict. 2. "--pull-request flag requires --commit-directly to be provided as well". This is why --pr-id was originally coupled to commit-directly, so the preceding commit that promoted --pr-id to plain --auto-pr runs was wrong and has been reverted. The assertion in test-analyze-args.yml is inverted to match: --pr-id must NOT appear on an --auto-pr-only run. Also adds cli-smoke checks that track both constraints. They emit notices rather than failing, since if the CLI relaxes a constraint nothing breaks - the action's guard just becomes unnecessarily strict and should be revisited. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * docs: correct the platform-support caveat with measured facts The previous caveat was partly wrong. Verified against the published 1.4.57 linux-x64 package and by running it in containers: - the binary is dynamically linked (interpreter /lib64/ld-linux-x86-64.so.2, needs libc.so.6 and libstdc++.so.6) and requires glibc >= 2.28, so RHEL/CentOS 7 and Ubuntu 18.04 are excluded too. That was not documented. - on Alpine, npm still installs the glibc linux-x64 package, because the platform packages carry no npm `libc` constraint. The failure therefore surfaces as `spawnSync .../mobbdev ENOENT` at spawn time. There is no helpful "install mobbdev instead" message, contrary to what the previous wording claimed. - on glibc < 2.28 the error is `mobbdev: not found`. In both cases the file exists; the ELF interpreter or libc does not. - the documented fallback, `npx mobbdev@latest`, was confirmed working on node:20-alpine. Replaces the prose caveat with a platform-support table and the real error signatures, so someone hitting either error can recognise it. Also notes that an Alpine container job cannot use this action regardless, since actions/setup-node installs official nodejs.org builds and has no musl variant; those users should invoke the CLI directly. Adds a cli-smoke musl-fallback job that runs in node:20-alpine and asserts the documented fallback still works, so the escape hatch cannot rot. It also emits a notice if @mobb.ai/cli ever starts working on musl. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * fix(ci): install PyYAML into a setup-python interpreter Running the lint workflow locally through `act` surfaced a real failure, not an emulation artifact: error: externally-managed-environment × This environment is externally managed `python3 -m pip install pyyaml` cannot install into the system Python on modern Ubuntu (PEP 668), which is what ubuntu-latest now is. Add actions/setup-python so pip has an interpreter it owns. Verified: both lint jobs now pass under act. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * docs: state supported runners instead of cataloguing unsupported ones Reframes the platform section around what the action supports: GitHub-hosted ubuntu runners, self-hosted Linux with glibc 2.28+, and the platforms the CLI publishes binaries for. Drops the table of unsupported environments and the error-signature notes. Keeps one neutral line pointing at `npx mobbdev@latest` for other environments, so anyone outside the supported set still has a path forward. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * refactor: let the Mobb CLI handle error and input conditions Trims this branch back to the migration itself, per review feedback. review/action.yml - restore the original logic: conditional -f and --scanner appends, and `eval ... || true`. Removes the fail-fast validation of report-file, scanner, commit-hash and pr-number, the scanner allowlist, and the fail-on-error input. Bugsy reports these conditions itself. action.yml - remove the create-one-pr/auto-pr and create-one-pr/commit-directly input validation for the same reason. both - remove the src-path input. These actions always run in GitHub CI, where the source is checked out at the workspace root, so pointing the CLI at an arbitrary local path is not a case worth supporting. - remove the MOBB_ACTION_DRY_RUN test seam and the mobb-command output. CI should exercise the real thing; API usage is not a constraint here. workflows - delete test-analyze-args.yml and test-review-args.yml, which existed only to assert the dry-run command string. - test-analyze-e2e.yml now covers the combinations for real: scan-and-fix, polling, scan-only, diff-aware, and a manual-only auto-pr leg. - test-review-e2e.yml drops fail-on-error and the negative job. The review action keeps its non-blocking behaviour, so the fix-report-url assertion is what turns a CLI problem into a red check in CI. - cli-smoke.yml drops the checks that only existed to justify the removed guards, and the musl fallback job. README - point everything at v1.1, the active release tag, and drop the discussion of other tags. - remove src-path and fail-on-error. Two small fixes are kept in both actions: quoting $OUT in the tr pipeline so CLI output containing * cannot glob, and the startsWith('https://') guard on the root action's status step, which the review action already had. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs * docs: update version references from v1.1 to v1.2 Point all action references and examples to the current active release tag. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverts PRs #31 and #33 to restore the original action code and unblock deployments.
The security fix (removing eval, using bash arrays) broke the autofixer CI test which uses
sedto replacenpx --yes mobbdev@latestwith a local build command containing an inline env var prefix (API_URL=...). This pattern is incompatible with bash arrays.The security fix will be re-applied with a different approach that is compatible with the test tooling.
Reverted commits
bf76c59— fix: use env command for array execution (fix: use env command for bash array execution [E-1815] #33)a12bce4— fix: prevent shell injection via eval (fix: prevent shell injection via eval in action.yml and review/action.yml [E-1815] #31)Ref: E-1815