ci: run the dependency audit on dev pull requests by jaylfc · Pull Request #2189 · jaylfc/taOS

jaylfc · 2026-07-28T08:49:43Z

The gap

security.yml triggers on push and pull_request to master only. Every exec PR targets dev, and master receives the work later at a release promotion.

So the dependency audit has never run on a pull request. Confirmed against the live queue rather than inferred:

all 23 open PRs target dev
no audit or security check is reported on tsk-gqsuah [OPEN] Gate: catch a retrofit put in MIGRATIONS where bas #2188 or fix(lists): add ProjectListsStore and fix ProjectListEntriesStore #2183

A vulnerable dependency introduced in a PR could merge into dev and ship in a beta with nothing having looked at it. The weekly cron still covers master, but after the fact and on the branch the work has already left.

The change

One line: pull_request.branches becomes [master, dev]. Nothing is renamed, so no required-context is orphaned, and the dependency-audit job itself is untouched.

Verified before enabling

pip-audit --ignore-vuln CVE-2026-3219 against the dev environment reports no known vulnerabilities, so this goes green on dev today and does not block the open queue.

Why now

taos-website hit the same class from the other direction: its audit read requirements.txt and missed the file that actually carried the test dependency, and the pinned pytest turned out to carry PYSEC-2026-1845.

taOS is not exposed to that CVE (pytest pinned >=9.1.1, above the 9.0.3 fix) and its audit does install the dev extras via pip install -e ".[dev,proxy,worker]". So our gap is the trigger, not the coverage.

Summary by CodeRabbit

Chores
- Expanded security auditing so pull requests targeting both the master and dev branches are covered.
- Improved workflow documentation to clarify branch-specific security expectations and the intended auditing coverage across development and releases.

security.yml triggered on push and pull_request to master only. Every exec PR targets dev, and master receives it later at a release promotion, so the dependency audit never ran on a pull request at all. Confirmed on the live queue: all 23 open PRs target dev, and no audit or security check is reported on any of them. The effect is that a vulnerable dependency could be introduced in a PR, merge into dev and ship in a beta with nothing having looked at it. The weekly cron still covers master, but only after the fact and on the branch the work has already left. Verified before enabling so this does not block the open queue: pip-audit against the dev environment reports no known vulnerabilities, so the job goes green on dev today. Prompted by taos-website hitting the same class from the other direction, where the audit read requirements.txt and missed the file that actually carried the test dependency, and a pinned pytest turned out to carry PYSEC-2026-1845. taOS is not exposed to that one (pytest is pinned >=9.1.1, above the 9.0.3 fix) and its audit does install the dev extras, so the gap here is the trigger rather than the coverage.

coderabbitai · 2026-07-28T08:50:07Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0cfc2cec-88b4-43cb-b2f8-9ef440b75004

📥 Commits

Reviewing files that changed from the base of the PR and between 4082bbd and 874b2e0.

📒 Files selected for processing (1)

.github/workflows/security.yml

🚧 Files skipped from review as they are similar to previous changes (1)

.github/workflows/security.yml

📝 Walkthrough

Walkthrough

The security workflow now runs on pull requests targeting both master and dev, and updates concurrency cancellation behavior for master and dev activity.

Changes

Security workflow coverage

Layer / File(s)	Summary
Expand security audit triggers `.github/workflows/security.yml`	The pull request trigger includes `master` and `dev`, while concurrency cancellation is configured for `master` and `dev` references with explanatory comments.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: enabling dependency audit runs for pull requests targeting dev.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ci/audit-dev-prs

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.}

gitar-bot · 2026-07-28T08:50:13Z

Gitar is working

_Gitar

kilo-code-bot · 2026-07-28T08:51:29Z

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)

.github/workflows/security.yml

Previous Review Summary (commit 4082bbd)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit `4082bbd`)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)

.github/workflows/security.yml

_{Reviewed by step-3.7-flash · Input: 119.5K · Output: 11.9K · Cached: 242.8K}

qodo-code-review · 2026-07-28T08:53:03Z

PR Summary by Qodo

CI: run dependency audit on dev pull requests

⚙️ Configuration changes ✨ Enhancement 🕐 Less than 10 minutes

AI Description

• Run the Security workflow on PRs targeting dev as well as master.
• Ensure dependency vulnerabilities are caught before merging into dev/betas.
• Document the rationale and verification to avoid blocking the current PR queue.

Diagram

graph TD
  PR(("PR to dev")) --> WF["Security workflow\nsecurity.yml"] --> JOB[["dependency-audit job"]] --> AUDIT["pip-audit"] --> DB[("Vulnerability DB")]
  PUSH(("Push to master")) --> WF
  CRON(("Weekly cron")) --> WF

  subgraph Legend
    direction LR
    _evt(("Trigger")) ~~~ _wf["Workflow"] ~~~ _job[["Job"]] ~~~ _db[("DB")]
  end

High-Level Assessment

The minimal change (adding dev to pull_request.branches) is the best approach: it preserves the existing workflow/job name (avoiding orphaned required checks) while closing the coverage gap for the branch where PRs actually land. Alternatives like splitting workflows per branch or switching to pull_request_target add complexity and/or security risk without improving audit coverage.

Files changed (1) +8 / -1

Other (1) +8 / -1

security.ymlTrigger Security workflow for dev pull requests +8/-1

Trigger Security workflow for dev pull requests

• Expands the pull_request trigger from master-only to include dev so dependency-audit runs on the branch where PRs merge. Adds inline documentation explaining the rationale and noting pre-verification that the audit is currently green on dev.

.github/workflows/security.yml

qodo-code-review · 2026-07-28T08:57:37Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 35 rules

✅ Skills: taos-development-skill

1. ~~Redundant audit runs~~ ✓ Resolved 🐞 Bug ➹ Performance

Description

By adding dev to the pull_request branch filter, the Security workflow will now run on nearly
every PR update but it has no concurrency cancellation, so superseded runs for the same PR may
continue running or remain pending instead of being canceled. This can waste runner minutes and
increase CI latency/noise compared to the repo’s existing pattern in ci.yml.

Code

.github/workflows/security.yml[14]
+    branches: [master, dev]

Relevance

⭐⭐⭐ High
Team has accepted CI/runtime efficiency tweaks; adding concurrency cancellation is low-risk and
reduces redundant runs.
PR-#391
PR-#379

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

security.yml now runs on PRs targeting dev, but unlike the main CI workflow it does not define
concurrency cancellation; therefore, repeated PR updates can leave redundant Security runs
active/pending rather than being auto-canceled.

.github/workflows/security.yml[3-23]
.github/workflows/ci.yml[21-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Security workflow now runs on PRs targeting `dev`, but it lacks a `concurrency` block. This means multiple pushes to the same PR can leave multiple redundant `dependency-audit` runs active/pending, consuming runner capacity and increasing turnaround time.

## Issue Context
The repo already uses concurrency cancellation in `.github/workflows/ci.yml` to avoid stacked runs for the same PR ref. The Security workflow should apply a similar approach, at least for `pull_request` events.

## Fix Focus Areas
- .github/workflows/security.yml[1-23]

## Suggested change
Add a `concurrency:` block (e.g., after `permissions:`) such as:

```yaml
concurrency:
 group: security-${{ github.workflow }}-${{ github.ref }}
 cancel-in-progress: ${{ github.event_name == 'pull_request' }}
```

This keeps scheduled/push runs intact while canceling superseded PR runs when new commits are pushed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Folding qodo's finding on #2189, verified against the tree: ci.yml carries a concurrency block and security.yml did not. That gap was harmless while the audit only ran on master, and started to matter the moment this PR made it run on every dev PR update, since each push would otherwise leave an orphaned audit running. Same guard as ci.yml: pushes to master and dev are never cancelled so the branch record stays complete; PR runs cancel their superseded predecessors.

jaylfc · 2026-07-28T09:35:45Z

Merging on my own verification, stated explicitly: no bot has reviewed this head. The concurrency fold created a new commit and there are zero bot comments after it, so the earlier ticks on this PR cover the previous tree, not this one.

What I verified rather than what CI reported:

The fold is qodo's finding, checked before folding. It said the Security workflow now runs on nearly every PR update with no concurrency cancellation, unlike ci.yml. Confirmed against the tree: ci.yml carries a concurrency block, security.yml did not. That gap was harmless while the audit ran on master only, and started to matter the moment this PR widened the trigger. Folded with the same guard as ci.yml, so pushes to master and dev are never cancelled and the branch record stays complete, while PR runs cancel their superseded predecessors.

The change proves itself. dependency-audit ran on this PR and passed. I had predicted it would not, on the belief that pull_request uses the base branch's workflow definition. That was wrong, and the correction is in the PR's favour: the new trigger is verifiable before merge rather than only after.

Verified green before proposing, so it does not block the queue. pip-audit --ignore-vuln CVE-2026-3219 against the dev environment reported no known vulnerabilities, and CI has now confirmed the same on the real matrix.

Required checks all present and green: test (3.12), test (3.13), lint, spa-build, plus dependency-audit. Nothing failing.

Nothing is renamed by this change, so no required context is orphaned.

* ci: run the dependency audit on dev pull requests security.yml triggered on push and pull_request to master only. Every exec PR targets dev, and master receives it later at a release promotion, so the dependency audit never ran on a pull request at all. Confirmed on the live queue: all 23 open PRs target dev, and no audit or security check is reported on any of them. The effect is that a vulnerable dependency could be introduced in a PR, merge into dev and ship in a beta with nothing having looked at it. The weekly cron still covers master, but only after the fact and on the branch the work has already left. Verified before enabling so this does not block the open queue: pip-audit against the dev environment reports no known vulnerabilities, so the job goes green on dev today. Prompted by taos-website hitting the same class from the other direction, where the audit read requirements.txt and missed the file that actually carried the test dependency, and a pinned pytest turned out to carry PYSEC-2026-1845. taOS is not exposed to that one (pytest is pinned >=9.1.1, above the 9.0.3 fix) and its audit does install the dev extras, so the gap here is the trigger rather than the coverage. * ci: cancel superseded security runs, matching ci.yml Folding qodo's finding on jaylfc#2189, verified against the tree: ci.yml carries a concurrency block and security.yml did not. That gap was harmless while the audit only ran on master, and started to matter the moment this PR made it run on every dev PR update, since each push would otherwise leave an orphaned audit running. Same guard as ci.yml: pushes to master and dev are never cancelled so the branch record stays complete; PR runs cancel their superseded predecessors.

qodo-code-review Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread .github/workflows/security.yml

jaylfc merged commit d728dd8 into dev Jul 28, 2026
18 checks passed

hognek mentioned this pull request Jul 28, 2026

fix(store): fail-closed canonicalisation + TOCTOU re-verify hardening #2186

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: run the dependency audit on dev pull requests - #2189

ci: run the dependency audit on dev pull requests#2189
jaylfc merged 2 commits into
devfrom
ci/audit-dev-prs

jaylfc commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

gitar-bot Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

kilo-code-bot Bot commented Jul 28, 2026 •

edited

Loading

Previous review (commit `4082bbd`)

Uh oh!

qodo-code-review Bot commented Jul 28, 2026

Uh oh!

qodo-code-review Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

jaylfc commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jaylfc commented Jul 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The gap

The change

Verified before enabling

Why now

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

gitar-bot Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kilo-code-bot Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Previous review (commit 4082bbd)

Uh oh!

qodo-code-review Bot commented Jul 28, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

jaylfc commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jaylfc commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

gitar-bot Bot commented Jul 28, 2026 •

edited

Loading

kilo-code-bot Bot commented Jul 28, 2026 •

edited

Loading

Previous review (commit `4082bbd`)

qodo-code-review Bot commented Jul 28, 2026 •

edited

Loading