Skip to content

Restore transitive soundness advisory reporting - #3137

Merged
Eliah Kagan (EliahKagan) merged 1 commit into
GitoxideLabs:mainfrom
EliahKagan:codex/run-ci/report-transitive-unsound
Oct 29, 2026
Merged

Eliah Kagan (EliahKagan) merged 1 commit into
GitoxideLabs:mainfrom
EliahKagan:codex/run-ci/report-transitive-unsound

Conversation

@EliahKagan

@EliahKagan EliahKagan commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Note

This description was written by GPT-6.1 Sol using the Codex harness at Eliah Kagan's direction.

An advisory-coverage review across GitoxideLabs, prompted by recent RustSec findings, revealed that gitoxide had silently lost reporting for transitive soundness advisories. Cargo-deny changed its default to unsound = "workspace"; gitoxide inherited that default through an action update. Explicitly setting unsound = "all" restores the earlier coverage. Soundness advisories can be security-relevant even when a concrete exploit has not been established.

The same commit updates the affected locked dependencies, so enabling the broader check does not introduce a commit with known advisory failures:

Dependency Update Advisory
event-listener 5.4.1 → 5.4.2 RUSTSEC-2026-0221
lru 0.18.0 → 0.18.2 RUSTSEC-2026-0253
tokio 1.52.3 → 1.53.3 RUSTSEC-2026-0386

The updates satisfy existing manifest constraints and gitoxide's MSRV. Cargo-deny continues to check its selected dependency graph; this setting does not make it audit every lockfile entry.

Validation: all four cargo-deny checks pass with --workspace --all-features --locked, using cargo-deny 0.20.2 and a copied RustSec database from October 29. Tix's 808 tests and the blocking/reqwest transport configuration's 54 tests pass; the separate async-only transport configuration passes 24 tests. These checks exercise SHA-1, reqwest/rustls, DNS, and async transport support. Local tests ran outside the sandbox after a macOS filesystem-watcher test timed out inside it and passed outside it.

The fork advisory job passes with the complete change. A separate fork-only test branch changes only deny.toml, retaining the original lockfile: its advisory job reports all three findings and fails with exit code 1. Both use the unchanged pinned CI action and workflow. The test commit is not intended for integration.

On the PR's commit, fork CI has passed Linux tests, macOS tests, and the Windows MSRV check. Other fork CI jobs were still running when this PR was opened.

Investigating advisory coverage across GitoxideLabs exposed that
`cargo-deny` now defaults to checking soundness advisories only for
direct workspace dependencies. Gitoxide inherited that narrower default
through an action update, losing coverage it previously had.

Set `unsound = "all"` explicitly so transitive soundness findings are
reported too. These findings can be security-relevant even when no
concrete exploit has been established.

Update the locked `event-listener`, `lru`, and `tokio` versions to
address `RUSTSEC-2026-0221`, `RUSTSEC-2026-0253`, and
`RUSTSEC-2026-0386`, respectively. Keep the dependency updates with the
policy change so this commit passes the expanded advisory check
independently.

Co-authored-by: Codex <codex@openai.com>

@EliahKagan EliahKagan left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

While Dependabot security updates are working, soundness advisories only receive them if they have been imported into the GitHub Security Advisory Database, which I think often does not happen if they are not known or strongly suspected to be security vulnerabilities (even perhaps when they do turn out to be vulnerabilities).

In contrast, Dependabot version updates are currently broken here in the gitoxide repository: the job always times out. Normally those would update transitive dependencies in Cargo.lock monthly, which was in effect the way we avoided having lots of unsound transitive-only dependencies that were readily patched.

While I think we should fix Dependabot version updates, I see scanning for soundness advisories in transitive-only dependencies using our existing cargo deny setup--just as we do in other crates in our dependency tree--to be a good thing.

I plan to enable auto-merge once the job has completed a run on the pull_request trigger, though I expect it to work since it worked in my fork on the push trigger.

Comment thread deny.toml Resolved
@EliahKagan
Eliah Kagan (EliahKagan) marked this pull request as ready for review October 29, 2026 22:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T22:28:32.997549Z abedf60 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@EliahKagan
Eliah Kagan (EliahKagan) merged commit 30a0daa into GitoxideLabs:main Oct 29, 2026
33 checks passed
@EliahKagan
Eliah Kagan (EliahKagan) deleted the codex/run-ci/report-transitive-unsound branch October 29, 2026 22:28

@EliahKagan EliahKagan left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This PR contains the questionable practice of hand-editing lock file entries that cargo wrongly changed back to the way they were. The lock file validates, but this is probably still worse than just tolerating the churn. This is the windows-sys thing that constantly vexes us. I forced Codex to not hand-edit the lockfile in GitoxideLabs/GixPython#6 (review), but I didn't intercede here. Sorry about that. I think this is acceptable in the extremely minimal sense that it doesn't need another PR right now just to fix it, but I'd be happy to open one on request.

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.

1 participant