Repository navigation
ci(core): add dependency-purity allowlist gate for gitlawb-core - #241
Merged
Merged
Conversation
gitlawb-core is embedded by gl, git-remote-gitlawb, and the node daemon, so it must stay lean. Add a CI gate that fails if its normal (non-dev, non-build) transitive dependency tree gains any crate not on an explicit allowlist. The allowlist is exhaustive-by-construction: a new heavy dependency reds CI whether or not anyone thought to ban it, which a denylist cannot do. The checker resolves like the build/test jobs (no --locked, since the committed lockfile can lag the manifests) and snapshots/restores Cargo.lock so a local run leaves no working-tree side effects. - ci/gitlawb-core-allowed-deps.txt: the 83 current normal deps, plus the regeneration command and rationale in the header - scripts/check-gitlawb-core-deps.sh: recompute and diff; hard-fail on a non-allowlisted crate, note-only on a stale (removed) allowlist entry - pr-checks.yml: new hard-fail core-deps-purity job, matching the pinned action SHAs and style of the sibling jobs
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a documented dependency allowlist and validation script for Changesgitlawb-core dependency purity
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI as core-deps-purity CI job
participant Script as check-gitlawb-core-deps.sh
participant Cargo as cargo tree
participant Allowlist as gitlawb-core-allowed-deps.txt
CI->>Script: Run dependency purity check
Script->>Cargo: Compute normal gitlawb-core dependencies
Script->>Allowlist: Read permitted crate names
Script-->>CI: Report success or fail on offenders
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ci/gitlawb-core-allowed-deps.txt`:
- Line 11: Update the allowlist comment describing new dependency behavior to
replace “reds CI” with “turns CI red,” preserving the rest of the wording
unchanged.
In `@scripts/check-gitlawb-core-deps.sh`:
- Around line 38-46: Update the current and allowed dependency command
substitutions in the script to avoid grep -v pipelines that return failure when
all lines are filtered under pipefail; use filtering that preserves successful
empty output. Also update the summary count near the dependency comparison to
report zero for an empty current value instead of counting it as one.
- Around line 39-42: Add --target all to the cargo tree command in
scripts/check-gitlawb-core-deps.sh and to the regeneration command documented in
ci/gitlawb-core-allowed-deps.txt, ensuring both dependency allowlist checks
include every Cargo target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 639077bc-907e-4220-97d6-092dd8e675a5
📒 Files selected for processing (3)
.github/workflows/pr-checks.ymlci/gitlawb-core-allowed-deps.txtscripts/check-gitlawb-core-deps.sh
| # wired into the `core-deps-purity` job in .github/workflows/pr-checks.yml) | ||
| # fails if gitlawb-core's tree contains any crate not listed here. | ||
| # | ||
| # The allowlist is exhaustive-by-construction: a NEW dependency reds CI whether |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the dependency-gate wording.
reds CI should be rewritten as turns CI red for grammatical correctness.
Proposed fix
-# of construction: a NEW dependency reds CI whether
+# of construction: a NEW dependency turns CI red whether🧰 Tools
🪛 LanguageTool
[style] ~11-~11: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...-construction: a NEW dependency reds CI whether # or not anyone thought to ban it, which a denyl...
(WHETHER)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ci/gitlawb-core-allowed-deps.txt` at line 11, Update the allowlist comment
describing new dependency behavior to replace “reds CI” with “turns CI red,”
preserving the rest of the wording unchanged.
Source: Linters/SAST tools
Comment on lines
+38
to
+46
| current="$( | ||
| cargo tree -p gitlawb-core --edges normal --prefix none --manifest-path "$ROOT/Cargo.toml" \ | ||
| | sed -E 's/ v[0-9].*$//' \ | ||
| | grep -v '^gitlawb-core$' \ | ||
| | sort -u | ||
| )" | ||
|
|
||
| # Allowlist with comments and blank lines stripped. | ||
| allowed="$(grep -vE '^[[:space:]]*(#|$)' "$ALLOW" | sort -u)" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '--- file outline ---\n'
git ls-files scripts/check-gitlawb-core-deps.sh
printf '\n--- relevant lines ---\n'
cat -n scripts/check-gitlawb-core-deps.sh | sed -n '1,140p'
printf '\n--- pipeline behavior probe ---\n'
tmpdir="$(mktemp -d)"
cat >"$tmpdir/probe.sh" <<'SH'
#!/usr/bin/env bash
set -o pipefail
probe() {
label="$1"
shift
if out="$("$@")"; then
printf '%s: ok, out=%q, status=%s\n' "$label" "$out" "$?"
else
st=$?
printf '%s: fail, out=%q, status=%s\n' "$label" "$out" "$st"
fi
}
probe grep_all_filtered bash -lc "printf 'a\nb\n' | grep -v . | cat"
probe grep_some_left bash -lc "printf 'a\nb\n' | grep -v '^a$' | cat"
probe sed_all_filtered bash -lc "printf 'a\nb\n' | sed '/./d' | cat"
probe count_empty bash -lc "current=''; if [ -n \"\$current\" ]; then dep_count=\"\$(printf '%s\n' \"\$current\" | wc -l | tr -d ' ')\"; else dep_count=0; fi; printf 'dep_count=%s\n' \"\$dep_count\""
probe count_nonempty bash -lc "current='x
y'; if [ -n \"\$current\" ]; then dep_count=\"\$(printf '%s\n' \"\$current\" | wc -l | tr -d ' ')\"; else dep_count=0; fi; printf 'dep_count=%s\n' \"\$dep_count\""
SH
bash "$tmpdir/probe.sh"Repository: Gitlawb/node
Length of output: 241
🏁 Script executed:
set -euo pipefail
cat -n scripts/check-gitlawb-core-deps.sh | sed -n '1,140p'
python3 - <<'PY'
import subprocess, textwrap, os, tempfile, pathlib
def run(cmd):
p = subprocess.run(cmd, shell=True, executable='/bin/bash', stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
print(f"$ {cmd}\nstatus={p.returncode}\n{p.stdout}")
run("bash -lc \"set -o pipefail; printf 'a\\nb\\n' | grep -v . | cat\"")
run("bash -lc \"set -o pipefail; printf 'a\\nb\\n' | grep -v '^a$' | cat\"")
run("bash -lc \"set -o pipefail; printf 'a\\nb\\n' | sed '/./d' | cat\"")
run("bash -lc \"current=''; if [ -n \\\"$current\\\" ]; then dep_count=\\\"$(printf '%s\\n' \\\"$current\\\" | wc -l | tr -d ' ')\\\"; else dep_count=0; fi; printf 'dep_count=%s\\n' \\\"$dep_count\\\"\"")
PYRepository: Gitlawb/node
Length of output: 3942
🏁 Script executed:
python3 - <<'PY'
import subprocess, textwrap
cases = [
("grep pipeline, all filtered", r"""bash -lc 'set -euo pipefail; current="$(printf "gitlawb-core\n" | grep -v "^gitlawb-core$" | sort -u)"; printf "current=%q\n" "$current"'"""),
("grep pipeline, one dep remains", r"""bash -lc 'set -euo pipefail; current="$(printf "gitlawb-core\nfoo\n" | grep -v "^gitlawb-core$" | sort -u)"; printf "current=%q\n" "$current"'"""),
("count empty current", r"""bash -lc 'current=""; printf "%s\n" "$current" | wc -l | tr -d " "'"""),
]
for label, cmd in cases:
p = subprocess.run(cmd, shell=True, executable="/bin/bash", stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
print(f"== {label} ==")
print(f"status={p.returncode}")
print(p.stdout)
PYRepository: Gitlawb/node
Length of output: 291
Avoid grep -v in these command substitutions. With set -o pipefail, current/allowed can fail when the filter removes every line, and the summary count prints 1 for an empty current value. scripts/check-gitlawb-core-deps.sh:38-46,71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check-gitlawb-core-deps.sh` around lines 38 - 46, Update the current
and allowed dependency command substitutions in the script to avoid grep -v
pipelines that return failure when all lines are filtered under pipefail; use
filtering that preserves successful empty output. Also update the summary count
near the dependency comparison to report zero for an empty current value instead
of counting it as one.
Comment on lines
+39
to
+42
| cargo tree -p gitlawb-core --edges normal --prefix none --manifest-path "$ROOT/Cargo.toml" \ | ||
| | sed -E 's/ v[0-9].*$//' \ | ||
| | grep -v '^gitlawb-core$' \ | ||
| | sort -u |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files scripts/check-gitlawb-core-deps.sh ci/gitlawb-core-allowed-deps.txt
printf '\n== script excerpt ==\n'
cat -n scripts/check-gitlawb-core-deps.sh | sed -n '30,55p'
printf '\n== allowlist excerpt ==\n'
cat -n ci/gitlawb-core-allowed-deps.txt | sed -n '1,40p'Repository: Gitlawb/node
Length of output: 3420
🌐 Web query:
cargo tree --target all default host target dependencies documentation
💡 Result:
The cargo tree command in Rust is used to visualize the dependency graph of a project [1][2]. By default, when no target is specified, cargo tree displays the dependencies for the host platform [1][3]. The --target flag allows you to filter the dependency graph based on a specific target triple [1][4]. When you provide the value all—specifically using --target all—cargo tree includes all platform-specific dependencies in the graph, regardless of the host platform [1][2]. This is particularly useful for identifying dependencies that are only pulled in when building for specific operating systems or architectures (e.g., dependencies guarded by platform-specific cfg attributes) [5].
Citations:
- 1: https://doc.rust-lang.org/stable/cargo/commands/cargo-tree.html
- 2: https://doc.rust-lang.org/cargo/commands/cargo-tree.html
- 3: https://manpages.debian.org/unstable/cargo/cargo-tree.1.en.html
- 4: https://man.archlinux.org/man/cargo-tree.1.en
- 5:
cargo tree -dskips dups shown by--target allbut that impact current target rust-lang/cargo#13175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== cargo tree help (if available) ==\n'
cargo tree --help | sed -n '1,220p'Repository: Gitlawb/node
Length of output: 142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [Path("scripts/check-gitlawb-core-deps.sh"), Path("ci/gitlawb-core-allowed-deps.txt")]:
print(f"\n### {p}")
text = p.read_text()
for needle in ["--target all", "cargo tree -p gitlawb-core", "Regenerate (from repo root"]:
print(needle, "=>", needle in text)
PYRepository: Gitlawb/node
Length of output: 142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== references ==\n'
rg -n "gitlawb-core-allowed-deps|check-gitlawb-core-deps|cargo tree -p gitlawb-core" -S .
printf '\n== package metadata relevant to target-specific deps ==\n'
cat -n Cargo.toml | sed -n '1,220p'Repository: Gitlawb/node
Length of output: 142
Include all Cargo targets in the dependency allowlist check.
cargo tree here only walks the host platform, so target-specific crates can bypass the allowlist. Add --target all to the checker and the regeneration command in ci/gitlawb-core-allowed-deps.txt.
📍 Affects 2 files
scripts/check-gitlawb-core-deps.sh#L39-L42(this comment)ci/gitlawb-core-allowed-deps.txt#L19-L23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check-gitlawb-core-deps.sh` around lines 39 - 42, Add --target all to
the cargo tree command in scripts/check-gitlawb-core-deps.sh and to the
regeneration command documented in ci/gitlawb-core-allowed-deps.txt, ensuring
both dependency allowlist checks include every Cargo target.
Source: MCP tools
beardthelion
added
crate:core
Proc-macro helper for curve25519-dalek (already allowlisted). Newer cargo versions list it under `cargo tree --edges normal` while older ones do not; the allowlist is a superset so it passes on both. Caught by core-deps-purity on the first CI run of this branch.
kevincodex1
approved these changes
Jul 22, 2026
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.
gitlawb-coreis embedded bygl,git-remote-gitlawb, and the node daemon, so it needs to stay lean. This adds a CI gate that fails if its normal (non-dev, non-build) transitive dependency tree gains any crate not on an explicit allowlist.The allowlist is exhaustive-by-construction: a new heavy dependency reds CI whether or not anyone thought to ban it, which a denylist can't do.
Files
ci/gitlawb-core-allowed-deps.txt: the 83 current normal deps, with the regeneration command and rationale in the header.scripts/check-gitlawb-core-deps.sh: recompute the tree and diff it, hard-fail on a non-allowlisted crate, note-only on a stale (removed) allowlist entry..github/workflows/pr-checks.yml: a new hard-failcore-deps-purityjob, mirroring the pinned action SHAs and style of the sibling jobs.Verification
Ran the gate end to end: green on the current tree, red when
tokiois added as a normal dependency ofgitlawb-core(it namestokioplus its transitive crates and exits non-zero), green again after reverting. The checker snapshots and restoresCargo.lock, so a local run leaves no working-tree side effects.Note on
--lockedThe checker deliberately does not pass
--locked. The committedCargo.lockcurrently lags the manifests (any plain cargo command refreshes it), so--lockedwould red this gate for lockfile-staleness reasons unrelated to core's dependencies. Refreshing the committed lockfile is a separate change.Summary by CodeRabbit