I built BayesCI a Bayesian GitHub Action for debugging failed C/C++ builds. Looking for feedback

[austinkilla228-art]

🏷️ Discussion Type

Question

💬 Feature/Topic Area

Workflow Configuration

Discussion Details

Hey GitHub Community! 👋

I'm developing BayesCI, an open-source GitHub Action that uses Bayesian ranking to identify suspicious commits after failed C/C++ builds.

It analyzes GCC, Clang, CMake and linker diagnostics, compares Git diffs, and displays ranked candidates in the Actions Summary.

The computational engine is written in C, with a replaceable Python analysis layer.

I've published the Action to GitHub Marketplace and tested it across repositories.

I'd appreciate feedback from GitHub Actions developers on two questions:

1. Is workflow_run the best approach for automatically analyzing failed CI builds?
2. What security or compatibility issues should I prioritize when supporting third-party repositories?

The project follows KISS and Install and Forget principles.

It's MIT-licensed, and contributions from beginners and experienced developers are welcome.

GitHub: https://github.com/austinkilla228-art/BayesCI

Thanks for any technical feedback!

[NexzaDev]

Hi @austinkilla228-art,

Interesting project. I read through the README, action.yml and the Python modules (I haven't run it), so the comments below are based on the code as it is on main right now.

1. Is workflow_run the right approach?

It's a reasonable default for this use case, and the way you've used it is mostly the safe way. Trade-offs to be aware of:

• Pros: the analysis runs after the CI run has finished, in a separate workflow, so it can use its own minimal permissions and the full run logs are available.
• Cons: GitHub only starts a workflow_run workflow whose file is on the default branch, and it runs against the default branch, so it can't be tested on a feature branch before merging. It's also coupled to the CI workflow's display name (workflows: ["CI"]), which breaks silently if someone renames it.
• Security model: workflow_run runs in the base repository's context, with its secrets and a read/write token, and it also fires for CI runs from fork pull requests. Your if: guard on event == 'push' avoids that, which is the right call. If you later add pull request support, be careful about this.

An alternative worth documenting is a final job in the same workflow with needs: [build] and if: failure(). It avoids the name coupling and works the same for pull requests, at the cost of users editing every CI workflow. Offering both modes (workflow_run as the "install and forget" path, needs for those who prefer it) would cover most users.

2. Security: what you already do well

• Inputs reach the shell through env: and not by interpolating ${{ }} directly into run:, which avoids script injection.
• permissions are actions: read and contents: read, persist-credentials: false is set, and the code from the failed checkout is never executed.
• The Git calls use argument lists without shell=True, and git show uses --no-ext-diff.
• The step summary only contains validated SHAs and numbers, not raw log text, which keeps Markdown and HTML injection out.

3. Security and robustness items I'd fix first

• shell=True in git_reader.get_ci_log. It runs a command from a YAML config. I don't see it being called from action_runner.py, but it ships in the same module, so I'd move it to a development-only file or remove it. It also forces import yaml at module level, which is why the action has to pip install PyYAML.
• Unpinned runtime dependency. pip install PyYAML in action.yml fetches from PyPI on every run with no version or hash, although the README says the Python code uses only the standard library. Removing the dependency (lazy import or drop it) is the simplest fix; otherwise pin the version.
• Pin your own action references. Pin actions/setup-python to a full commit SHA, as you'd recommend to your users, and keep it updated with Dependabot.
• Validate SHAs before passing them to Git. head_sha and the API values are trustworthy in practice, but a ^[0-9a-f]{40}$ check is cheap defense in depth, and prevents an unexpected value from being parsed as an option.
• Token and redirects. get_failed_log sends Authorization with urllib, and the logs endpoint redirects to a pre-signed storage URL. As far as I know, urllib keeps custom headers on redirects, so the bearer token may be forwarded to the other host. Please check this, and consider handling the redirect manually without the header.
• Resource limits. The run-logs ZIP is read fully into memory with no size cap. For big matrix builds this can be large, so add a size limit and a clear message when it's exceeded.
• Errors. In get_commits_since, check=True raises a raw traceback if the last good SHA isn't an ancestor (for example after a force-push or rebase). Catch it and write a friendly summary instead.
• Dead code. gtet_commits_since contains typos (chek_output, undefined repo_path) and would fail if called, so remove it.

4. Compatibility issues to prioritize for third-party repos

• Runners. The action compiles the C engine with make at run time. That's fine on GitHub-hosted Ubuntu and macOS runners, but it will fail on Windows runners and on minimal self-hosted or container jobs. Either check runner.os and fail with a clear message, ship prebuilt binaries per OS, or provide a pure-Python fallback.
• Last successful run lookup. get_successful_runs reads only the first 100 successful runs, filtered by branch. Add pagination, filter by event as well (otherwise successful pull request runs on the same branch name can be matched), and think about reruns and cancelled or timed-out conclusions, since you currently only trigger on failure.
• Logs. The run-logs archive includes every job, including successful ones, which adds noise in matrix builds. Fetching the logs of the failed jobs only (via the jobs API) usually gives cleaner evidence.
• Log parsing realities. Actions logs have timestamp prefixes, may contain ANSI colour codes (compilers with colour diagnostics), and use absolute runner paths such as /home/runner/work/<repo>/<repo>/src/file.c. Strip the prefixes and escape codes, and normalize paths to repository-relative ones before matching against diffs. MSVC and localized compiler messages are another gap.
• Git history. You document fetch-depth: 0, which is right. A clear error when the history is shallow would help, as would handling submodules and very large commit ranges.
• Fewer inputs. The five identifying inputs can be read from GITHUB_EVENT_PATH inside the action, which cuts the YAML users must copy and matches your "install and forget" goal.

5. Packaging and release

The README refers to @v1.0.0, but the repo has no release yet, so publish a tag (and a moving v1), and recommend pinning to a full SHA. Run the end-to-end test from a separate repository that the status section mentions, using a fixture repo with deliberately failing commits and a matrix of OSes and compilers. Adding a SECURITY.md is also a good idea for an action that processes other people's logs.

6. If you extend the summary later

Keep untrusted strings (commit messages, file names, raw log lines) out of the summary and stdout, or escape them. Lines starting with :: in stdout can be interpreted as workflow commands, and Markdown from a repository can be used to make a summary misleading.

Hope this is useful, and good luck with the release!

[austinkilla228-art]

Hi @NexzaDev thank you for taking time to actually read through the code!
This is exactly the kind of technical feedback I was hoping to get
I'll prioritize reviewing the token handling during redirects, removing the unnecessary shell=True development code, and adding resource limits for downloaded logs.
I also agree that simplifying the action inputs would fit the project's "Install and Forget" philosophy really well.
One small update: BayesCI v1.0.0 has now been released on GitHub Marketplace, and I've already tested the Action from a separate repository.
I'll keep the C computational core simple while improving security and compatibility around it.
Your review was really detailed I really appreciate it I will take your suggestion to add SECURITY.md

[ilhamwahyuj]

1. Is workflow_run the best approach?

• Yes, for decoupled analysis: workflow_run is the standard, cleanest pattern if you want "install-and-forget" automation that triggers on completed with conclusion == 'failure'. It keeps diagnostic logic completely out of the primary build workflow.
• Alternative (always() / failure() composite action): For third-party repos that don't want a separate workflow file, providing a step run with if: failure() allows direct access to build artifacts and environment without needing cross-workflow artifact downloads.

---

2. Priority Security & Compatibility Considerations

• Pwn-request vulnerabilities on workflow_run: Since workflow_run runs in the context of the default branch with read/write token privileges by default, never untar, execute, or evaluate untrusted code/build scripts from fork PRs. Treat downloaded build logs and diffs strictly as untrusted text strings.
• Token permissions (Least Privilege): Ensure documentation explicitly requests minimal permissions:

permissions:
  actions: read      # download failed logs/artifacts
  contents: read     # inspect diffs/commits
  pull-requests: write # or checks:write / summary output

• Log availability & masking: Raw build logs may be truncated or sanitized. Prefer having users pipe compiler warnings/errors to an explicit artifact file (e.g., -fdiagnostics-format=json for GCC/Clang) rather than parsing raw Actions console stdout.
• Compiler diagnostic stability: Different compiler versions and custom CMake wrapper formats format error messages differently; prioritize standardized machine-readable outputs (like GCC JSON output or SARIF format) over fragile regex matching.

[austinkilla228-art]

Hi @ilhamwahyuj
Thanks for the detailed feedback!
I agree that workflow_run is the better fit for BayesCI's "Install and Forget" philosophy.
Security is definitely a priority. The goal is to analyze Git diffs and build logs as untrusted data without executing code from the failed revision.
For now, I'll keep token permissions limited to actions: read and contents: read, since BayesCI only publishes results to the Actions job summary.
I also really like your suggestion about machine-readable GCC/Clang diagnostics. That's something I'd like to explore in future versions while keeping the C computational core simple.

Really appreciate your input!