Skip to content

[fix] Fixed context scope to prevent hallucinations#607

Merged
nemesifier merged 13 commits intomasterfrom
fix/ci-failure-bot
Mar 5, 2026
Merged

[fix] Fixed context scope to prevent hallucinations#607
nemesifier merged 13 commits intomasterfrom
fix/ci-failure-bot

Conversation

@stktyagi
Copy link
Member

@stktyagi stktyagi commented Mar 5, 2026

Previous method of fetching logs using gh view run was prone to weird context, whereas now it limits context and ensures it is correct.

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Description of Changes

Changed fetching logs step to be both correct and concise using gh api endpoints and filtering runs to jobs and jobs to steps.

Previous method of fetching logs using gh view run was prone to weird context, whereas now it limits context and ensures it is correct.
@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an actor input to the reusable CI-failure workflow and its callers, removes the gemini_model workflow input (model now sourced from workflow vars), updates trusted_scripts checkout ref to fix/ci-failure-bot, refactors log collection to enumerate failed jobs and steps producing failed_logs.txt, and passes ACTOR into the AI analysis environment. The Python analyzer reads ACTOR from env, generates a scoped tag_id for marking untrusted failure/code blocks, and injects a conditional {greeting} into the system/user prompt.

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActor as GitHub Actor
  participant ReusableWF as Reusable Workflow
  participant TrustedRepo as trusted_scripts
  participant LogFetcher as Log Collector
  participant AIJob as analyze_failure.py
  participant GitHubAPI as GitHub API

  GitHubActor->>ReusableWF: trigger workflow (input: actor)
  ReusableWF->>TrustedRepo: checkout ref=fix/ci-failure-bot
  ReusableWF->>LogFetcher: enumerate failed jobs & steps
  LogFetcher->>ReusableWF: failed_logs.txt
  ReusableWF->>AIJob: run analysis (env: ACTOR, GEMINI_MODEL from vars)
  AIJob->>GitHubAPI: post report/comment (uses greeting and tag_id)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • nemesifier
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title clearly identifies the main change: fixing context scope to prevent hallucinations in the CI failure bot's log fetching mechanism, which aligns with the core objective of replacing an overly broad log-fetching method with a more precise approach.
Description check ✅ Passed The description covers the main change and references the new log-fetching implementation, but lacks details on why the old method was problematic and leaves the test/documentation checklist items unchecked despite the template requirements.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/ci-failure-bot

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@stktyagi
Copy link
Member Author

stktyagi commented Mar 5, 2026

Previously fetched logs were: -

UNKNOWN STEP 2026-03-05T07:13:23.8398652Z 2d79b6e2292f92064ed9ecf04a825199caee885b Python==3.11 | django~=4.2.0 
UNKNOWN STEP 2026-03-05T07:13:23.8402234Z ##[endgroup] Python==3.11 | django~=4.2.0 
UNKNOWN STEP 2026-03-05T07:13:23.8403757Z ##[group]Determining the checkout info Python==3.11 | django~=4.2.0 
UNKNOWN STEP 2026-03-05T07:13:23.8405051Z ##[endgroup] Python==3.11 | django~=4.2.0 
UNKNOWN STEP 2026-03-05T07:13:23.8407808Z [command]/usr/bin/git sparse-checkout disable Python==3.11 | django~=4.2.0 
UNKNOWN STEP 2026-03-05T07:13:23.8441376Z [command]/usr/bin/git config --local --unset-all extensions.worktreeConfig Python==3.11 | django~=4.2.0 
UNKNOWN STEP 2026-03-05T07:13:23.8462833Z ##[group]Checking out the ref

this included each and every log line whether it failed or not and without it step headline.

the updated method now obtains final logs in this form and only for failed steps from failed jobs:-

===== JOB 65928733571 =====
2026-03-05T19:40:02.5988033Z ##[group]Run echo "Traceback (most recent call last):"
2026-03-05T19:40:02.5989055Z �[36;1mecho "Traceback (most recent call last):"�[0m
2026-03-05T19:40:02.5989980Z �[36;1mecho "  File \"test.py\", line 10, in <module>"�[0m
2026-03-05T19:40:02.5990970Z �[36;1mecho "ValueError: Something went terribly wrong"�[0m
2026-03-05T19:40:02.5991793Z �[36;1mexit 1�[0m
2026-03-05T19:40:02.6050539Z shell: /usr/bin/bash -e {0}
2026-03-05T19:40:02.6051142Z env:
2026-03-05T19:40:02.6051752Z   pythonLocation: /opt/hostedtoolcache/Python/3.10.19/x64
2026-03-05T19:40:02.6052989Z   PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.10.19/x64/lib/pkgconfig
2026-03-05T19:40:02.6054210Z   Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.19/x64
2026-03-05T19:40:02.6055280Z   Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.19/x64
2026-03-05T19:40:02.6056558Z   Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.19/x64
2026-03-05T19:40:02.6057684Z   LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.10.19/x64/lib
2026-03-05T19:40:02.6058539Z ##[endgroup]
2026-03-05T19:40:02.6549459Z Traceback (most recent call last):
2026-03-05T19:40:02.6558988Z   File "test.py", line 10, in <module>
2026-03-05T19:40:02.6560014Z ValueError: Something went terribly wrong
2026-03-05T19:40:02.6571158Z ##[error]Process completed with exit code 1.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/actions/bot-ci-failure/analyze_failure.py:
- Around line 100-107: The actor fallback currently uses actor =
os.environ.get("ACTOR", pr_author) which doesn't handle ACTOR set to an empty
string; change the logic that sets actor so an empty or whitespace-only ACTOR
falls back to pr_author (e.g., use actor_val = os.environ.get("ACTOR"); actor =
actor_val.strip() if actor_val and actor_val.strip() else pr_author) and keep
the existing greeting construction that compares pr_author.lower() to
actor.lower() to choose the appropriate greeting string.

In @.github/workflows/reusable-bot-ci-failure.yml:
- Line 54: The workflow currently pins the actions checkout ref to the feature
branch value 'fix/ci-failure-bot' (the ref key in the
reusable-bot-ci-failure.yml), which will break after the branch is deleted;
update the ref value to the repository's stable branch (e.g., 'master' or your
main branch name) or to a durable reference (like the default branch variable)
so the workflow continues to run after this PR is merged.
- Around line 108-109: The commands that set START_CLEAN and END_CLEAN use echo
$START and echo $END without quoting, which can cause word-splitting or
globbing; update the two assignments (symbols START_CLEAN and END_CLEAN) to
quote the variables when passing them to echo (i.e., use echo "$START" and echo
"$END") so the full values are preserved and safe from shell expansion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b49b63ca-6a27-4033-ab58-38c999384662

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc3be2 and 029d95f.

📒 Files selected for processing (3)
  • .github/actions/bot-ci-failure/analyze_failure.py
  • .github/workflows/bot-ci-failure.yml
  • .github/workflows/reusable-bot-ci-failure.yml
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
📚 Learning: 2026-03-05T14:23:49.735Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

Applied to files:

  • .github/workflows/bot-ci-failure.yml
  • .github/workflows/reusable-bot-ci-failure.yml
🔇 Additional comments (5)
.github/actions/bot-ci-failure/analyze_failure.py (1)

146-147: LGTM!

The {greeting} f-string substitution correctly integrates the conditional greeting logic into the system instruction, maintaining the expected response format with the dynamic header and commit SHA reference.

.github/workflows/reusable-bot-ci-failure.yml (3)

24-26: LGTM!

The new actor input is properly defined as required and correctly wired to the ACTOR environment variable at line 130. This aligns with the analyze_failure.py script which reads ACTOR from the environment (defaulting to PR_AUTHOR if not provided).


132-132: Ensure vars.GEMINI_MODEL is configured.

GEMINI_MODEL is now sourced from repository/organization variables instead of workflow inputs. If vars.GEMINI_MODEL is not defined, this will be empty, which may cause the Python script to use a default model or fail.

Verify that vars.GEMINI_MODEL is configured in the repository or organization settings where this workflow will be used. If the variable might not exist, consider providing a fallback:

GEMINI_MODEL: ${{ vars.GEMINI_MODEL || 'gemini-pro' }}

110-117: The awk filter assumption is correct. GitHub Actions logs are formatted with ISO8601 timestamps as the first field on each line (e.g., 2024-01-08T21:41:07.4620669Z message), exactly matching the script's expectation. The filter correctly extracts the timestamp, removes fractional seconds via sub(/\..*/, "", ts), and performs string-based comparison, which works correctly for ISO8601 format timestamps.

.github/workflows/bot-ci-failure.yml (1)

66-66: LGTM!

The new actor input correctly passes github.actor to the reusable workflow, satisfying the newly required input parameter. This enables the downstream analyze_failure.py script to differentiate between the PR author and the workflow trigger actor for appropriate greeting logic.

Fixed actor empty case and greetings
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please also increase security as suggested by coderabbit here?
#584 (comment)

Improved security by moving tag upwards so attacker cannot guess.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/reusable-bot-ci-failure.yml (1)

54-54: ⚠️ Potential issue | 🔴 Critical

Use a durable checkout ref before merge.

Line [54] still pins trusted_scripts to a feature branch. After merge/branch cleanup, this breaks the workflow; use master (or another durable ref).

🐛 Proposed fix
-          ref: fix/ci-failure-bot
+          ref: master
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/reusable-bot-ci-failure.yml at line 54, The workflow
currently pins the checkout ref for trusted_scripts to a short-lived feature
branch ('fix/ci-failure-bot'), which will break after the branch is deleted;
update the ref value used for trusted_scripts in
.github/workflows/reusable-bot-ci-failure.yml (the checkout/uses entry that
references trusted_scripts) to a durable branch or tag such as 'master' (or
main/another long-lived ref) so the workflow continues working after
merges/cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/reusable-bot-ci-failure.yml:
- Around line 83-84: The JOB_IDS assignment only fetches the default first page
of jobs; change the gh command used to fetch jobs so it paginates and returns
all failed job ids. Replace the current JOB_IDS command invocation with the
paginated API call (use gh api --paginate repos/$REPO/actions/runs/$RUN_ID/jobs
with the same -q filter that selects .conclusion=="failure" and .id) so the
JOB_IDS variable includes failures from all pages; ensure the command still
assigns into JOB_IDS and preserves the existing -q filter expression.

---

Duplicate comments:
In @.github/workflows/reusable-bot-ci-failure.yml:
- Line 54: The workflow currently pins the checkout ref for trusted_scripts to a
short-lived feature branch ('fix/ci-failure-bot'), which will break after the
branch is deleted; update the ref value used for trusted_scripts in
.github/workflows/reusable-bot-ci-failure.yml (the checkout/uses entry that
references trusted_scripts) to a durable branch or tag such as 'master' (or
main/another long-lived ref) so the workflow continues working after
merges/cleanup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 46e95730-becd-460a-a756-8a8922cdf1da

📥 Commits

Reviewing files that changed from the base of the PR and between c06ff89 and b54cff6.

📒 Files selected for processing (2)
  • .github/actions/bot-ci-failure/analyze_failure.py
  • .github/workflows/reusable-bot-ci-failure.yml
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
📚 Learning: 2026-03-05T09:38:03.232Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T14:23:49.735Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
  • .github/actions/bot-ci-failure/analyze_failure.py
🔇 Additional comments (4)
.github/actions/bot-ci-failure/analyze_failure.py (2)

100-107: Good defensive actor handling and greeting branching.

The fallback on Line [100] correctly handles empty/whitespace ACTOR, and the case-insensitive branch on Lines [104]-[107] avoids duplicate mentions cleanly.


109-120: Prompt hardening with scoped untrusted blocks looks solid.

Using tag_id (Line [109]) to scope untrusted sections and explicitly instructing the model to ignore in-band directives (Lines [115]-[120]) is a strong mitigation for prompt-injection attempts.

Also applies to: 155-155

.github/workflows/reusable-bot-ci-failure.yml (2)

24-26: actor input contract is correctly added as required.

This aligns with downstream env usage and prevents missing-context runs.


130-132: Runtime env wiring for analysis is consistent.

Passing ACTOR and sourcing GEMINI_MODEL from workflow vars is consistent with the analyzer’s runtime resolution/fallback logic.

correct fix using the -f flag for query parameters, not URL embedding
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/reusable-bot-ci-failure.yml:
- Around line 92-99: The gh api calls that populate FAILED_STEPS and write job
logs (the commands that set FAILED_STEPS via gh api
repos/$REPO/actions/jobs/$JOB_ID and gh api .../logs > job_logs.txt) can fail
under set -e and abort the whole step; wrap those calls so an API hiccup doesn't
exit (e.g., run them with a safe fallback like appending "|| true" or capture
the exit code and handle non-zero without exiting), then test for empty/invalid
FAILED_STEPS or job_logs.txt before proceeding and continue to the next job when
appropriate; update the logic around the FAILED_STEPS variable and the gh api
.../logs invocation to tolerate failures and still append "===== JOB $JOB_ID
=====" and any available partial logs to failed_logs.txt.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b0d97313-9860-4902-9d7a-c803cfdf159f

📥 Commits

Reviewing files that changed from the base of the PR and between b54cff6 and 6bdf1f8.

📒 Files selected for processing (1)
  • .github/workflows/reusable-bot-ci-failure.yml
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
📚 Learning: 2026-03-05T14:23:49.735Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:38:03.232Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
🔇 Additional comments (3)
.github/workflows/reusable-bot-ci-failure.yml (3)

24-26: Actor propagation is correctly wired end-to-end.

actor is required at Line 24 and passed to runtime env at Line 130; this aligns with the workflow_call contract.

Also applies to: 130-130


54-54: Keep this as a pre-merge reminder: switch off feature-branch ref.

At Line 54, ref: fix/ci-failure-bot is still pinned to a PR branch, so this remains the same concern already raised earlier.


83-84: Pagination fix looks good.

Using --paginate with per_page=100 at Lines 83-84 correctly avoids dropping failed jobs in large/matrix runs.

@stktyagi
Copy link
Member Author

stktyagi commented Mar 5, 2026

  • DONE: Update docs

@coveralls
Copy link

coveralls commented Mar 5, 2026

Coverage Status

coverage: 97.25%. remained the same
when pulling 084fcc6 on fix/ci-failure-bot
into 5fc3be2 on master.

stktyagi added 2 commits March 5, 2026 23:17
temporary debugging for testing
fixed endpoint for fetching logs
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
.github/workflows/reusable-bot-ci-failure.yml (1)

101-107: ⚠️ Potential issue | 🟠 Major

API failures will abort the entire log extraction.

With set -eo pipefail (Line 90), failures in the gh api calls at Lines 101-102 or 107 will terminate the step early, skipping partial logs for remaining jobs. This concern was raised in a previous review but remains unaddressed.

Consider wrapping these calls to continue processing other jobs on failure:

🐛 Proposed fix
           for JOB_ID in $JOB_IDS; do
             echo "Processing job $JOB_ID"
-            FAILED_STEPS=$(gh api repos/$REPO/actions/jobs/$JOB_ID \
-              -q '.steps[] | select(.conclusion=="failure") | `@base64`')
+            if ! FAILED_STEPS=$(gh api repos/$REPO/actions/jobs/$JOB_ID \
+              -q '.steps[]? | select(.conclusion=="failure") | `@base64`'); then
+              echo "Could not fetch steps for job $JOB_ID; skipping." >> failed_logs.txt
+              continue
+            fi
             if [ -z "$FAILED_STEPS" ]; then
               echo "No failed steps in job $JOB_ID"
               continue
             fi
-            gh api repos/$REPO/actions/jobs/$JOB_ID/logs > job_logs.txt
+            if ! gh api repos/$REPO/actions/jobs/$JOB_ID/logs > job_logs.txt; then
+              echo "Could not fetch logs for job $JOB_ID; skipping." >> failed_logs.txt
+              continue
+            fi
             echo "===== JOB $JOB_ID =====" >> failed_logs.txt
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/reusable-bot-ci-failure.yml around lines 101 - 107, The gh
API calls that populate FAILED_STEPS and fetch job logs can cause the script to
exit due to set -eo pipefail; wrap the two gh api invocations (the one producing
FAILED_STEPS via "gh api repos/$REPO/actions/jobs/$JOB_ID -q ..." and the one
writing job_logs.txt via "gh api repos/$REPO/actions/jobs/$JOB_ID/logs") with
error-tolerant handling so a failing API call does not abort processing of
subsequent jobs—e.g., run them in a guarded conditional or append a fallback (or
|| true) to capture the failure, log the error, and continue to the next job;
ensure you still detect empty FAILED_STEPS and skip log download when
appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/reusable-bot-ci-failure.yml:
- Around line 101-107: The gh API calls that populate FAILED_STEPS and fetch job
logs can cause the script to exit due to set -eo pipefail; wrap the two gh api
invocations (the one producing FAILED_STEPS via "gh api
repos/$REPO/actions/jobs/$JOB_ID -q ..." and the one writing job_logs.txt via
"gh api repos/$REPO/actions/jobs/$JOB_ID/logs") with error-tolerant handling so
a failing API call does not abort processing of subsequent jobs—e.g., run them
in a guarded conditional or append a fallback (or || true) to capture the
failure, log the error, and continue to the next job; ensure you still detect
empty FAILED_STEPS and skip log download when appropriate.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ae0b82d7-6857-4862-8adc-b76a21f630f0

📥 Commits

Reviewing files that changed from the base of the PR and between 6bdf1f8 and 1362613.

📒 Files selected for processing (1)
  • .github/workflows/reusable-bot-ci-failure.yml
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
📚 Learning: 2026-03-05T14:23:49.735Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:38:03.232Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
🔇 Additional comments (5)
.github/workflows/reusable-bot-ci-failure.yml (5)

24-26: LGTM!

The new actor input is properly defined and aligns with the downstream usage in analyze_failure.py, which safely falls back to pr_author if the actor value is empty.


54-54: Reminder: Revert ref to master before merging.

This was flagged in a previous review and acknowledged by the maintainer. The feature branch fix/ci-failure-bot will be deleted after merge, breaking this workflow.


119-127: LGTM!

The AWK-based timestamp filtering correctly handles ISO 8601 timestamps using lexicographic comparison. The null completed_at fallback at Line 115 properly handles steps that may still be running or have missing completion times.


139-141: LGTM!

The ACTOR and GEMINI_MODEL environment variables are properly configured. The Python analyzer safely handles missing or empty values with appropriate fallbacks (pr_author for actor, gemini-2.5-flash-lite for model).


81-88: Good defensive validation.

The RUN_ID check with clear error messaging helps catch misconfigured caller workflows early. The debug group provides useful troubleshooting context.

@openwisp-companion
Copy link

Socket Address In Use Error

Hello @stktyagi,

(Analysis for commit 1362613)

It looks like your CI job failed due to a NS_ERROR_SOCKET_ADDRESS_IN_USE error. This means that a process is already using the network port (43943) that another process is trying to bind to.

Explanation:
The logs indicate that multiple attempts to start a server on port 43943 failed because the address was already in use. This is a common issue in CI environments where processes might not be cleaned up properly between jobs or within the same job.

Remediation:
Since this is an infrastructure-level error, the most straightforward solution is to re-run the CI job. This often resolves the issue by providing a clean environment.

If the problem persists after re-running the job, it might indicate a more persistent issue with the CI environment's process management. In such cases, further investigation into the CI runner's configuration or state would be necessary.

stktyagi added 3 commits March 6, 2026 00:22
Modified workflow to see context ingested
Test new technique for fetching
fixed invalid -f per_page flag causing 404
@stktyagi
Copy link
Member Author

stktyagi commented Mar 5, 2026

https://github.com/openwisp/bot-testing-ground/actions/runs/22733913482

Context is now limited and tested

@stktyagi
Copy link
Member Author

stktyagi commented Mar 5, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/reusable-bot-ci-failure.yml (1)

54-54: ⚠️ Potential issue | 🔴 Critical

Use a durable checkout ref before merge.

At Line 54, ref: fix/ci-failure-bot is branch-scoped and will break once that branch is deleted. Switch to a stable ref (e.g., master) before merging.

🐛 Proposed fix
-          ref: fix/ci-failure-bot
+          ref: master
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/reusable-bot-ci-failure.yml at line 54, The workflow
currently pins the checkout to a branch-scoped ref (`ref: fix/ci-failure-bot`);
replace that value in the reusable workflow's checkout step (the `ref` key) with
a durable stable ref such as `master` or `main` (or a release tag or commit SHA)
so the workflow won't break when the temporary branch is deleted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/reusable-bot-ci-failure.yml:
- Around line 89-95: The GH API call that populates JOB_IDS is unguarded and
will cause the script to exit under set -eo pipefail if gh fails; modify the
JOB_IDS assignment (the command invoking gh api --paginate
repos/$REPO/actions/runs/$RUN_ID/jobs -q '.jobs[] |
select(.conclusion=="failure") | .id') so failures are swallowed (e.g., append
|| true or use a conditional capture like if ! JOB_IDS=$(gh api ...); then
JOB_IDS=""; fi) so the later check if [ -z "$JOB_IDS" ] can run and produce
failed_logs.txt instead of aborting.

---

Duplicate comments:
In @.github/workflows/reusable-bot-ci-failure.yml:
- Line 54: The workflow currently pins the checkout to a branch-scoped ref
(`ref: fix/ci-failure-bot`); replace that value in the reusable workflow's
checkout step (the `ref` key) with a durable stable ref such as `master` or
`main` (or a release tag or commit SHA) so the workflow won't break when the
temporary branch is deleted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a5fefa48-711c-4660-a6de-131266b12f36

📥 Commits

Reviewing files that changed from the base of the PR and between 1362613 and b7564cf.

📒 Files selected for processing (1)
  • .github/workflows/reusable-bot-ci-failure.yml
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
📚 Learning: 2026-03-05T14:23:49.735Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:38:03.232Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.

Applied to files:

  • .github/workflows/reusable-bot-ci-failure.yml
🔇 Additional comments (2)
.github/workflows/reusable-bot-ci-failure.yml (2)

24-26: actor workflow input is correctly defined and required.

This matches the downstream ACTOR environment usage and avoids silent empty-context behavior.


132-134: Environment wiring for ACTOR and repo-level GEMINI_MODEL looks good.

The variable source change is consistent with reusable-workflow configuration centralization.

Updated documentation and add guards in ci
@stktyagi
Copy link
Member Author

stktyagi commented Mar 5, 2026

@coderabbitai check for any potential security issues

@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me examine the relevant files for potential security issues.


🏁 Script executed:

#!/bin/bash
echo "=== analyze_failure.py ==="
fd -e py analyze_failure --exec cat {}

echo "=== reusable-bot-ci-failure.yml ==="
fd reusable-bot-ci-failure.yml --exec cat {}

echo "=== bot-ci-failure.yml ==="
fd bot-ci-failure.yml --exec cat {}

Length of output: 89


🏁 Script executed:

#!/bin/bash
echo "=== Repo structure under .github ==="
find .github -type f | sort

echo ""
echo "=== analyze_failure.py ==="
find . -name "analyze_failure.py" -exec cat {} \;

echo ""
echo "=== reusable-bot-ci-failure.yml ==="
find . -name "reusable-bot-ci-failure.yml" -exec cat {} \;

echo ""
echo "=== bot-ci-failure.yml ==="
find . -name "bot-ci-failure.yml" -exec cat {} \;

Length of output: 15985


@stktyagi, here's a thorough security analysis of the changes in this PR:


🔒 Security Analysis

🔴 Critical — Hardcoded PR Branch for Trusted Script Checkout

File: reusable-bot-ci-failure.yml, Checkout Reusable Workflow step

- name: Checkout Reusable Workflow
  uses: actions/checkout@v6
  with:
    repository: openwisp/openwisp-utils
    ref: fix/ci-failure-bot   # ⚠️ PR branch, not master
    path: trusted_scripts

This is the most significant security issue. The workflow checks out trusted_scripts from the fix/ci-failure-bot feature branch rather than master. This means:

  • Any push to fix/ci-failure-bot by anyone with write access immediately affects what code runs in production with full access to GEMINI_API_KEY, APP_ID, and PRIVATE_KEY secrets.
  • Once this PR is merged, the branch may be deleted, breaking all downstream consumers of this reusable workflow.

Fix: Change to master before merging:

ref: master

For maximum supply-chain security, consider pinning to a specific commit SHA instead.


🟠 High — Expression Injection in Shell run: Steps

File: bot-ci-failure.yml, Find PR Number step

Script injection attacks can occur directly within a workflow's inline script. The expressions inside ${{ }} are evaluated and substituted with the resulting values before the shell script is run, which can make it vulnerable to shell command injection.

These patterns in the workflow are flagged:

PR_NUMBER="${{ github.event.workflow_run.pull_requests[0].number }}"
HEAD_SHA="${{ github.event.workflow_run.head_sha }}"

There are less obvious sources of potentially untrusted input, such as branch names and email addresses, which can be quite flexible in terms of their permitted content. For example, zzz";echo${IFS}"hello";# would be a valid branch name and would be a possible attack vector for a target repository.

While head_sha is always a hex string and PR numbers are numeric (making exploitation unlikely in practice), the recommended fix is to create an environment variable containing the untrusted value. The expression is resolved and then used as the value for that environment variable — nothing is injected into the generated file, and the value is treated as a string.

Fix:

- name: Find PR Number
  id: pr
  env:
    GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
    REPO: ${{ github.repository }}
    PR_NUMBER_FROM_PAYLOAD: ${{ github.event.workflow_run.pull_requests[0].number }}
    HEAD_SHA: ${{ github.event.workflow_run.head_sha }}
  run: |
    PR_NUMBER="$PR_NUMBER_FROM_PAYLOAD"
    ...
    PR_NUMBER=$(gh api repos/$REPO/commits/$HEAD_SHA/pulls ...)

🟡 Medium — Unpinned Action Versions (Supply Chain Risk)

File: reusable-bot-ci-failure.yml

uses: actions/create-github-app-token@v2
uses: actions/checkout@v6
uses: actions/setup-python@v6

Always pin to commit SHAs, not version tags. Mutable tags can be moved or overwritten; even verified publishers can get compromised — or their accounts taken over.

Fix: Pin each action to a full commit SHA:

uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4

You can find SHAs on each action's releases page.


🟡 Medium — CI Logs and Fork Code Sent to External API

File: analyze_failure.py, get_repo_context() and get_error_logs()

The code reads the full pr_code/ directory (checked out from inputs.head_repo, which is a potentially untrusted fork) and sends it — along with failed_logs.txt — to the Gemini API, an external Google service. Two risks arise:

  1. Secrets in CI logs: If any CI step accidentally echoes a secret value, it would be included in failed_logs.txt and forwarded to Google's API. Because there are multiple ways a secret value can be transformed, automatic redaction is not guaranteed.
  2. Fork code exfiltration: A malicious fork could include proprietary or sensitive files (e.g. .env files not in .gitignore) that would be read by get_repo_context() and sent externally.

Suggestions:

  • Add .env, *.pem, *.key, *.crt, *.p12 to the ignore_dirs/deny-list inside get_repo_context().
  • Document in the README that CI logs may be forwarded to the Gemini API.

🟢 Low — Weak Entropy for Prompt Injection Tag ID

File: analyze_failure.py

tag_id = secrets.token_hex(4)  # 32-bit entropy

The tag_id is used to scope untrusted data blocks inside the prompt (<failure_logs_{tag_id}>). With only 32 bits (~4 billion possibilities), a determined attacker with many CI runs could eventually guess the tag and craft content that breaks the isolation. This is a low-risk issue in practice, but trivially fixable:

tag_id = secrets.token_hex(16)  # 128-bit entropy — much stronger

🟢 Low — Greeting Embedded in System Instruction

File: analyze_failure.py

greeting = f"Hello @{pr_author},"
system_instruction = f"""...\n2. **Greeting**: {greeting}..."""

pr_author and actor are embedded directly into the system_instruction. GitHub usernames are restricted to alphanumeric + hyphens, so prompt injection through these values is very unlikely — but it's still cleaner to keep user-controlled strings exclusively in the prompt (inside the <..._tag_id> tagged blocks), not in the system_instruction.


Summary Table

Severity Finding File
🔴 Critical Hardcoded PR branch fix/ci-failure-bot for trusted scripts reusable-bot-ci-failure.yml
🟠 High Expression injection pattern in run: blocks bot-ci-failure.yml
🟡 Medium Unpinned action versions (@v2, @v6) reusable-bot-ci-failure.yml
🟡 Medium Fork code + CI logs sent to external Gemini API analyze_failure.py
🟢 Low 32-bit tag_id entropy for prompt injection isolation analyze_failure.py
🟢 Low greeting (user-controlled) embedded in system_instruction analyze_failure.py

The most urgent fix before merging is changing ref: fix/ci-failure-botref: master in the Checkout Reusable Workflow step.


🧠 Learnings used
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.

stktyagi added 3 commits March 6, 2026 02:02
Added checks for security
Removed debug statements from fetch ci logs
Updated ref to master and added comments for fetch step explanation
@nemesifier
Copy link
Member

@coderabbitai are you happy now?

@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

Let me check the current state of the files to see if the issues have been addressed.


🏁 Script executed:

#!/bin/bash
echo "=== reusable-bot-ci-failure.yml ==="
find . -name "reusable-bot-ci-failure.yml" -exec cat {} \;

echo ""
echo "=== bot-ci-failure.yml ==="
find . -name "bot-ci-failure.yml" -exec cat {} \;

echo ""
echo "=== analyze_failure.py ==="
find . -name "analyze_failure.py" -exec cat {} \;

Length of output: 15844


@nemesifier, mostly yes — the two most important issues have been resolved:

  • Critical fixed: ref: fix/ci-failure-botref: master in reusable-bot-ci-failure.yml.
  • High fixed: Expression injection mitigated — PR_NUMBER and HEAD_SHA are now passed through environment variables (PR_NUMBER_PAYLOAD, EVENT_HEAD_SHA) before being used in the shell script in bot-ci-failure.yml.
  • Medium partially addressed: get_repo_context() now skips .env* files and sensitive extensions (.pem, .key, .crt, .p12).

Three smaller items remain open if you'd like to address them:

Severity Finding Status
🟡 Medium Unpinned action versions (@v2, @v6) in reusable-bot-ci-failure.yml Open
🟢 Low secrets.token_hex(4) — 32-bit entropy for prompt-injection tag Open
🟢 Low greeting (user-controlled) embedded in system_instruction Open

The two low-severity items are essentially cosmetic hardening improvements. The unpinned actions are the only remaining item worth addressing before merging if supply-chain hygiene is a concern, but they're not blockers for the core functionality fix.


🧠 Learnings used
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.

Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.

Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

@nemesifier
Copy link
Member

@coderabbitai: we are happy with using version tags and master for openwisp-utils, we know what we're doing and are fine with that, so please if there's nothing else approve and keep this in mind for our org (stop bringing this up all the time).

@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

@nemesifier, understood — I'll stop flagging unpinned action versions and master/version-tag usage for openwisp-utils. The two remaining low-severity items (token_hex(4) entropy and greeting in system_instruction) are purely cosmetic hardening and not blockers. Happy to approve!


✏️ Learnings added
Learnt from: nemesifier
URL: https://github.com/openwisp/openwisp-utils/pull/607

Timestamp: 2026-03-05T20:55:29.947Z
Learning: For the openwisp/openwisp-utils repository (and the broader openwisp org), the maintainers have explicitly decided to use mutable version tags (e.g. `v2`, `v6`) for GitHub Actions and to use `ref: master` for internal script checkouts. Do NOT flag unpinned action versions or `master`/version-tag usage as security issues in any openwisp-utils workflow file. The maintainers are aware of the trade-offs and are intentionally fine with this approach.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:49.735Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.

Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:03.232Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
✅ Actions performed

Comments resolved and changes approved.

@nemesifier nemesifier merged commit 3996dbc into master Mar 5, 2026
35 checks passed
@nemesifier nemesifier deleted the fix/ci-failure-bot branch March 5, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants