* feat(core): add shared is_bidi_format predicate for terminal sanitizers (#183)
Terminal-bound error sanitizers must strip Cf bidi/format controls as well as Cc control bytes (INV-6), or a right-to-left override in attacker-influenceable bytes can visually reorder the displayed line (Trojan Source, CWE-451). gl already did this with a local is_bidi_format; two sibling sanitizers shipped with only the Cc half (#183). Centralize the reordering-subset predicate here so all consumers reuse one audited definition instead of re-deriving the set. Deliberately excludes legitimate Cf chars (ZWJ, soft hyphen).
* refactor(gl): consume shared is_bidi_format from gitlawb-core (#183)
Replace gl's local is_bidi_format copy with the centralized gitlawb-core predicate so exactly one definition exists in the workspace and the three terminal sanitizers cannot drift apart. Behavior-preserving; the existing strip test is unchanged and a must-not-over-strip guard (ASCII + Arabic U+0627 + ZWJ U+200D survive) pins the reordering-only scope.
* fix(git-remote): strip Cf bidi controls from terminal-bound error bodies (#183)
safe_error_body_excerpt filtered only Cc control bytes, so a Cf bidi override in a hostile or MITM'd node's HTTP error body reached the operator's terminal and could visually reorder the line (Trojan Source, CWE-451). Drop the reordering Cf subset via the shared gitlawb-core::sanitize::is_bidi_format, without setting pending_space so stripped chars leave no phantom gap. Covered per bidi class plus a transport-path proof through handle_connect, RED observed before the fix.
* fix(icaptcha): strip Cf bidi controls from terminal-bound error excerpts (#183)
sanitize_excerpt filtered only Cc, so a Cf bidi override in a node-advertised iCaptcha service's reason/body reached the terminal on the bail! path (Trojan Source, CWE-451). Drop the reordering Cf subset via the shared gitlawb-core predicate, without pending_space. The existing sanitize_strips_control_chars_and_escapes test asserted only is_control() and so was vacuous for this class (why the gap shipped); a dedicated per-class bidi test now covers it, RED observed before the fix.
* style: apply rustfmt to #183 sanitizer additions
* test(#183): pin a leading-position bidi char in the strip tests
The per-class strip tests placed bidi chars only mid-string. A bidi char at index 0 exercises the empty-excerpt/out path (where the pending_space guard behaves differently), confirming a leading override still yields no phantom leading space. Traced correct; now asserted.
* test(review): pin that bidi chars don't consume the length cap (#183)
Correctness, adversarial, and testing reviewers all flagged that no test locks in the drop-before-count ordering: a dropped bidi char must not consume MAX_ERR_CHARS budget. Behavior verified correct by execution during review; this test guards it against regression (interleaved bidi across the cap still yields exactly MAX_ERR_CHARS visible chars, no phantom spaces).
---------
Co-authored-by: t <t@t>
* fix(node,git-remote): gate receive-pack advertisement, sign client fetch/push
The info/refs ref advertisement was gated only for service=git-upload-pack
(the visibility check sat inside `if service == "git-upload-pack"`), so an
anonymous GET /{owner}/{repo}.git/info/refs?service=git-receive-pack returned a
private repo's branch names and commit tips, bypassing the read gate. Run the
visibility "/" check for both services.
Now that the advertisement is gated for both services, git-remote-gitlawb has to
authenticate its own requests, or a private repo's owner can neither fetch nor
push it. Sign the Phase-1 advertisement GET and the Phase-2 pack POST for both
services over path_and_query when an identity keypair is present; public repos
stay anonymous. This also repairs a pre-existing break where an owner could not
fetch their own private repo, because the upload-pack POST was sent unsigned.
Tests: node-side gate (anon denied, owner-signed cleared, both services); a
real-signature seam proving the client's own signature verifies under the node's
verification primitives; real served-content withholding against an on-disk repo
(the owner gets the ref bytes, a denied caller's 404 omits them); and mockito
coverage that the client attaches and omits signatures correctly.
* docs(node,git-remote): correct auth-contract comments on receive-pack gating (#119)
receive-pack POST is signature-required and separately owner-gated, not
visibility_check-gated; push implies read, not read-subset-of-push.
* test(node): assert unsigned receive-pack POST is rejected (#119)
No test covered the signature-required half of the receive-pack auth
contract. Wire the POST route behind require_signature as production does and
assert an unsigned POST returns 401 before reaching the handler; 401 (not the
handler's 404/500) proves require_signature rejected it pre-handler.
* test(node): RAII-clean the served-refs test's temp dirs on panic (#119)
advertisement_serves_real_refs_only_to_authorized_callers created /tmp/<slug>/<name>.git and /tmp/gl-seam-src-<short> and only removed them after the final assertion, leaking those dirs on the runner when an earlier assertion failed. Wrap each known path in a DirGuard whose Drop calls remove_dir_all, bound right after creation, so cleanup runs on success or panic. tempfile::TempDir does not fit here because repo_store::for_testing fixes the path layout.
* test(node): drop git_info_refs from KNOWN_UNGATED now that #119 gates it
The repo-scoped egress guard's allowlist (added in #122, merged to main
after this branch was opened) still listed git_info_refs as ungated. This
branch makes the info/refs visibility gate unconditional for both
git-upload-pack and git-receive-pack, so the handler is now gated and the
staleness assert requires removing it from the allowlist.
* style(node,git-remote): tidy #119 review nits (em dashes, dead binding, test header)
Post-rebase ce-code-review pass on #119. No behavior change:
- replace em dashes in the added comments with commas/colons/parentheses
- drop the dead `let _ = COVERED_COMPONENTS;` binding (and its import); the
`missing_components().is_empty()` assert already covers the intent
- add a section header above the new git-info-refs test block to match the
module's sectioning convention
* test(node): recurse nested api modules in the egress-gate guard
The completeness scan in every_repo_scoped_handler_is_gated used a flat
read_dir over src/api, so a future api/<module>/mod.rs could add a
repo-scoped handler the scrape never inspects. Walk the api tree recursively
via a new collect_rs_files helper (covered by its own unit test), skipping
only the top-level mod.rs so nested module files are checked too.
* fix(git-remote): keep public fetches anonymous, sign only on a 404 retry (#119)
git-remote-gitlawb signed every git-upload-pack request whenever an identity
existed, disclosing the caller's DID to any node it fetched a public repo from,
even though public repos need no auth. Fetch now sends the advertisement GET
anonymously and escalates to a single signed retry only when the node denies it
with 404 (how it withholds a private repo from an unauthenticated reader); if the
retry was needed, the upload-pack POST is signed too. Push (git-receive-pack) is
unchanged and signs from the first request, since a push can never be anonymous.
Verified against the node by execution: an unsigned private fetch returns 404,
not 401, and a signed retry clears the read gate for the owner and for a granted
non-owner reader, while a signed non-reader stays 404.
Tests drive handle_connect end to end (recording mock): public fetch stays
anonymous for GET and POST, private fetch retries signed once and signs the POST,
a keyless fetch does not retry, a still-denied signed retry surfaces 404 once, and
push signs from the first request.
* fix(node): scan nested api sources by relative path in the egress guard (#119)
The egress-gate completeness scan skipped already-covered files by basename
(listed.contains(fname)), so a nested api/<module>/repos.rs would be skipped
because the top-level repos.rs is listed, letting a future nested module add an
ungated repo-scoped handler the scrape never inspects. Extract the skip logic into
unlisted_source_files and match on the path relative to api_root, so only the exact
top-level files already covered are skipped and a nested file with a colliding
basename is still scanned. Completes CodeRabbit's outstanding request.
---------
Co-authored-by: t <t@t>
release-type: rust at path . is incompatible with this virtual
workspace root. release-please's Rust strategy unconditionally appends a
CargoToml update for the root Cargo.toml, and that updater throws
"is not a package manifest (might be a cargo workspace)" whenever the
manifest has no [package] section — so every run died on the final step.
Switch to release-type: simple (touches only a version file + CHANGELOG,
never a manifest) and bump each crate version through the generic updater
via extra-files plus an inline x-release-please-version annotation.
Preserves the single vX.Y.Z tag, all action outputs, and the entire
downstream release.yml unchanged.
release-please's Rust workspace strategy versions each member crate in its own
Cargo.toml and cannot rewrite an inherited `version.workspace = true`, so it
failed with 'value at path package.version is not tagged'. Give each of the five
crates an explicit version = "0.3.9" (kept in lockstep by release-please) and
drop the now-unused [workspace.package].version. Inter-crate deps are path-only,
so no version requirements need syncing.
* perf: trim allocations and redundant copies on hot paths
- Cargo.toml: enable thin LTO + symbol stripping for release builds
- git-remote-gitlawb: attach pack body after signing to move (not clone)
potentially large pack bytes, halving peak push memory
- gitlawb-attest: deserialize payload by reference instead of cloning it
- core/http_sig: compare components by &str to avoid per-item String alloc
- node/tigris: drop redundant to_vec() copy of the response body
- node/rate_limit: only allocate the key String on first insert, not per request
- node/sync: resolve the peer table once per batch instead of per item
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* perf(sync): skip peer lookup on empty sync batch
Return early from process_batch when dequeue_pending_syncs yields nothing
— the common case on the 30s idle poll — so an empty batch no longer fires
an unnecessary list_peers() DB query (CodeRabbit, PR #80).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* test(sync): cover per-batch peer origin-URL resolution
Extract the peer -> origin-URL lookup from process_batch into a pure
resolve_origin_url() helper and unit-test it (match + trailing-slash trim,
unknown DID, empty peer list). This is the per-batch resolution PR #80
introduced; pulling it out of the DB-bound process_batch lets it be tested
without Postgres. No behavior change. Addresses the needs-tests label.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* fix(tigris): use a unique temp dir per archive extraction
The fixed `.{file_name}.tmp-extract` path let two concurrent extractions of
the same repo share one temp dir, so one's remove_dir_all/unpack could clobber
the other's in-progress work. Suffix the temp dir with a fresh UUID so each
extraction is isolated; this also removes the need to pre-clear a leftover dir
(a unique name can't collide with an interrupted run). The on-failure cleanup
still removes our own dir, and success consumes it via rename (CodeRabbit).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* fix(tigris): serialize repo publish per path to close the swap race
Unique temp dirs let extractions unpack in parallel, but the final
remove_dir_all + rename swap could still interleave for the same local_path,
causing a nondeterministic overwrite or a rename-onto-non-empty failure. Guard
just the swap with a process-wide mutex keyed by local_path so only one
extraction publishes per repo at a time; the expensive unpack stays parallel
(CodeRabbit).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* docs(tigris): restore decompress_repo doc comment ordering
The earlier publish_lock insertion landed between decompress_repo's doc
comment and its fn, gluing the two doc blocks onto publish_lock and leaving
decompress_repo undocumented. Reorder so each fn keeps its own doc. No
behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* docs(tigris): note publish_lock map is never evicted
Document the known unbounded-growth limitation of the per-path LOCKS map:
bounded by the repo count so negligible for normal use, but high-volume
deployments may want LRU/weak-ref eviction. No behavior change (CodeRabbit).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
* ci: harden PR pipeline (audit, MSRV, release build, beta matrix, docker smoke)
Restructure the single stable-only fmt+clippy+test job into separate jobs:
fmt+clippy (stable), a test matrix on stable+beta (beta non-blocking),
build --release, cargo audit, an MSRV check pinned to Rust 1.91, and a
Docker build smoke test mirroring the release workflow. Add concurrency
cancellation for superseded PR runs and per-job timeouts.
Declare rust-version = "1.91" in [workspace.package] and inherit it in
every member crate so the stated MSRV is a real, CI-enforced contract.
* ci: disable credential persistence on checkout steps
Set persist-credentials: false on all six actions/checkout steps. These
jobs run untrusted PR code via cargo and docker build, and checkout's
default leaves the GITHUB_TOKEN in the local git config where that code
could read it. None of the jobs push or otherwise need git credentials.
* fix(deps): bump rustls-webpki to 0.103.13
Clears three advisories flagged by the new cargo audit job: a reachable
panic in CRL parsing and two name-constraint bypasses (wildcard and URI).
Semver-compatible patch, lockfile only, no manifest or code change.
cargo also corrects a stale lockfile reference while rewriting Cargo.lock:
data-encoding-macro-internal 0.1.17 (a build-time macro crate) requires
syn 1.x, and its dependency entry is re-pinned from syn 2.0.117 to the
1.0.109 it actually resolves to. Unrelated to the bump, unavoidable when
cargo touches the lock, and the more-correct state.
* ci(audit): suppress no-fix advisories with justification and a re-check
The cargo audit job stays hard-fail. The remaining three advisories have
no available upstream fix, so they are suppressed in .cargo/audit.toml
(the location cargo-audit actually reads; a repo-root audit.toml is not
read), each with an inline rationale and removal condition:
- RUSTSEC-2026-0118/0119 (hickory-proto 0.25.2, via libp2p-dns 0.44):
reachable, since the node wraps QUIC in the system DNS resolver. No fix
until libp2p-dns ships hickory 0.26. Accepted, reachable risk.
- RUSTSEC-2023-0071 (rsa Marvin): in the lockfile via sqlx's unconditional
sqlx-mysql, but not linked (sqlx is postgres-only), so no RSA oracle
exists; no upstream fix regardless.
Two controls keep the suppression honest. The PR audit job echoes the
active ignore list to the job summary, so a green check is not read as
advisory-clean. A weekly scheduled job re-runs cargo audit without the
ignores for visibility and hard-fails if hickory-proto moves off 0.25.x
while the ignores remain, so the suppression cannot silently outlive the
upstream fix.
* ci(audit): reference tracking issue #76 in the hickory ignores
* ci(audit): treat hickory-proto removal as drift too
The scheduled drift guard only failed when hickory-proto was present and
off 0.25.x, so removing it from Cargo.lock entirely left the RUSTSEC-2026-0118
/0119 ignores in place with the guard still passing. Treat an absent version
as drift as well, so dead ignore entries are caught whether the dependency is
bumped past 0.25.x or removed.
git-remote-gitlawb was the only workspace binary without `--version`, so
release smoke tests could not validate the packaged helper directly. This
is called out in docs/MAINTAINER-ROADMAP.md ("Add --version support to
git-remote-gitlawb so release smoke tests can validate it directly") and
docs/OSS-READINESS-AUDIT.md.
- `--version`/`-V` prints `git-remote-gitlawb <version>`, matching the
`<bin> <version>` format the clap-based `gl` and `gitlawb-node` emit so
release tooling can treat all three binaries uniformly. The version
comes from CARGO_PKG_VERSION.
- `--help`/`-h` prints concise usage (URL form, env vars, flags).
- Flags are only recognized as the first argument, so git's
`<remote-name> <url>` remote-helper invocation path is untouched; a
remote literally named "--version" is not mistaken for the flag.
- Informational flags are handled before tracing init so output stays
clean for scripts.
Adds unit tests for argument classification (including the no-args and
misplaced-flag edge cases) and the version/help strings.
Co-authored-by: FrozenRaspberry <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
* feat: embed bootstrap peers seed list for automatic network discovery
A fresh \`docker compose up\` now joins the Gitlawb network with zero
manual peer configuration. The node parses an embedded \`bootstrap-peers.json\`
on startup and merges the entries into both the HTTP gossip task and the
libp2p Kademlia bootstrap.
- Add \`bootstrap-peers.json\` at repo root (versioned schema, PR-friendly)
- New \`bootstrap\` module in the node crate (parse + merge_seeds)
- Wire into \`main\` after \`Config::parse\`
- Operators can opt out via \`GITLAWB_BOOTSTRAP_DISABLE_SEEDS\` for
isolated dev networks
Also in this commit:
- \`cargo fmt --all\` over the entire workspace (no logic changes)
- Downgrade CI clippy step to advisory (\`continue-on-error: true\`) until
the existing lint backlog is cleared. fmt + tests stay strict.
Co-Authored-By: OpenClaude <[email protected]>
* test: cover bootstrap merge logic with unit tests
Refactor bootstrap.rs into pure functions (parse_seed_list, merge_into_vecs)
so the parse + merge logic can be tested without constructing a Config or
mutating process-global env vars.
Adds 11 tests covering:
- valid v1 list parses
- unknown version is rejected
- malformed JSON is rejected
- empty / missing peers array
- merge appends new http + p2p entries
- merge dedupes against existing entries
- invalid p2p_multiaddr is skipped (http still added)
- empty strings are skipped
- null optional fields are tolerated
- the canonical bootstrap-peers.json shipped in the repo always parses
(regression guard against future schema changes)
Co-Authored-By: OpenClaude <[email protected]>
* fix(security): reject path traversal in repo_store local_path
Closes CodeQL alert #4 (Uncontrolled data used in path expression).
Both `owner_did` and `repo_name` come from URL parameters and were used
unsanitized to build a filesystem path, so a request like
`/did:key:foo/../../../etc/passwd.git/info/refs` could escape the repos
directory.
Fix:
- New `validate_owner_did` and `validate_repo_name` enforce a strict
allowlist before path construction (alphanumeric + `: . _ -` for DIDs;
alphanumeric + `. _ -` for repo names; rejects empty, `..`, leading
`.` or `-`, slashes, backslashes, null bytes, overlong inputs).
- `RepoStore::local_path` now returns `Result<…>` and the 5 callers
propagate the error.
- Defence in depth: even after the allowlist passes, the joined path is
checked to still be rooted at `repos_dir`.
- 17 unit tests cover normal DIDs/repo names plus every malicious shape
(`..`, `/`, `\`, leading dot/dash, null byte, overlong, did:web with
dots).
Co-Authored-By: OpenClaude <[email protected]>
* fix(security): add Path::components barrier for CodeQL rust/path-injection
CodeQL's rust/path-injection query did not recognise the existing
allowlist validator as a sanitiser, so it kept flagging local_path even
though every malicious input was already rejected.
Adds an explicit Path::components() walk after path construction that
rejects any ParentDir or CurDir segment. This is the barrier pattern
CodeQL recognises — and is also genuine defence-in-depth in case a
future change ever weakens the allowlist.
Allowlist + starts_with + components walk = three independent layers
between attacker-controlled HTTP input and filesystem access. All 17
existing tests still pass.
Co-Authored-By: OpenClaude <[email protected]>
---------
Co-authored-by: OpenClaude <[email protected]>
github-actions[bot]