Parsing of IPv6 addresses not enclosed in square brackets causes a
warning to be logged.
This is too strict, since the user can not influence IPv6 addresses
received over the network, but only those in their configuration.
Remove the warning from parsing, and instead carry a boolean to warn the
user later.
The invalid timestamp error will only tell the timestamp of the
offending node's message.
Currently, there are issues where that error is occurring but the
timestamp seems legitimate. This patch adds the running node's
timestamp to help debug the issue.
Move logging-related module `radicle::logging` into its own crate.
While at it, remove the "logger" feature flag from `radicle`.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.dev>
`cargo check` fails because `human-panic` and `sysinfo` require at least
1.88.0.
+++ command cargo check --release --locked --all-targets
error: rustc 1.85.0 is not supported by the following packages:
human-panic@2.0.6 requires rustc 1.88
sysinfo@0.37.2 requires rustc 1.88
Bump MSRV to fix this.
1.88.0 introduced [let chains], which in turn has `clippy` warn about
nested if statements. All of these sites are fixed in this change.
1.87.0 introduced [`is_multiple_of`], which is a more readable version
of `x % y == 0`.
[let chains]: https://blog.rust-lang.org/2025/06/26/Rust-1.88.0/#let-chains
[`is_multiple_of`]: https://doc.rust-lang.org/std/primitive.usize.html#method.is_multiple_of
In `cb57e6560fe7767e79b87473a11261fbb82ce84c` the loading of systemd
credentials was refactored. The idea was to only change the accepted IDs
slightly after switching from radicle.xyz to radicle.dev. Unfortunately,
this was done in a sloppy way. The two cases for passphrase and secret
actually differ in ways that was overlooked.
Rectify these mistakes and restore the old behavior:
1. The logic to read the passphrase from the file that contains the
credential is moved out of `load_credential` since it only applies
to the passphrase, not the secret.
2. The warnings printed are now generic, and will not mention the
passphrase in case reading the path for the secret fails.
- Fixes a bug where the log level set in the config file was
ignored: `Logger` and `StderrLogger` captured the level in a
`self.level` field at construction time and checked it in
`Log::enabled`. After config was loaded, the global
`log::set_max_level` was updated but `self.level` was not,
so verbose messages were dropped by the per-instance filter
even when the global filter allowed them.
- Make `log::set_max_level` the single source of truth: remove
the `level` field and have `Log::enabled` defer to
`log::max_level()` so the two filters can no longer drift.
- Update call sites to construct loggers without a level.
- Disable the structured logger's internal filter (set to "trace") so
that it also falls back to `log::set_max_level`.
Canonical references can not be used to model symbolic references.
Relax this restriction by adding another member to the payload, named
"symbolic". Its key-value/name-target pairs then translate directly to
canonical symbolic references.
Care is taken to not allow circular references, and that there always
is a rule that could generate the target of a symbolic reference
(or a chain of symbolic references). Still, a symbolic reference may
dangle (for example when its target reference cannot be computed
because of divergence), but at least it can be prevented that a
symref *always* dangles, because there is no rule that would produce
its target.
Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Previously `test_fetch_emits_canonical_ref_update` was passing by
catching a leftover `CanonicalRefUpdated` event from the initial
repository fetch. Furthermore, the new commit was written to the
un-namespaced root. It can be proven to be the case by commenting out
the `alice.commit_to(...)` line and watching the test still pass.
Fix the test by committing to the namespaced default branch which will
trigger the `CanonicalRefUpdated` that is expected by Bob. Drains events
so we don't catch the wrong event, and explicitly announces.
The `commit_to` method should namespace the reference, so that node is
only ever committing the namespace of that particular node.
This prevents the node from committing to another node's namespace or
the canonical namespace.
Previously a rule like `refs/heads/releases-*` would be rewritten
incorrectly to `refs/heads/releases-**/*` matching the 'trailing
asterisk' rule.
Simplify the rewrite rule so that a trailing `*` becomes a prefix
matching rule. Add tests.
Reorganises the rules test suite into a `test` module and a `helper`
sub-module.
The former holds the original tests, removing the `test_` prefix.
The latter holds helper functions for test setup.
- Drop `type='ipv6'` rows whose bracket-stripped inner part contains
no `:`, since every valid IPv6 textual form has at least one
- Cover the offender (`[]:8776`), bracketed garbage (`[abc]:8776`),
the unspecified (`::`), loopback, and full forms in tests
- Keep `dns` and `ipv4` rows untouched
Migration 8 retyped any `[..]:N` dns row to ipv6 by bracket pattern
alone, without checking the inner part is parseable
- Decode each row independently so one corrupt entry no longer aborts
the whole address-book iterator
- Log a warn with the raw `value` and continue past parse failures
- Add a regression test that mimics the post-migration-8 state where a
legacy `[]:8776` row lands as `type='ipv6'`
A pre-`df8e4e6c` parser admitted `[]:8776` as `HostName::Dns("[]")`.
Migration 8 retyped any `[..]:N` dns row to `ipv6` without validating
the inner part, so on read `Address::from_str` rejects the row with
"invalid IPv6 address syntax", which bubbled up and caused
`available_peers()` to silently return empty, breaking peer discovery
for the affected node.
Update the output of `rad_jj_colocated_patch` so that it passes with the current version of `jj` being used:
```
jj --version
jj 0.35.0
```
Mark `rad_jj_bare` as `ignore` since it currently cannot determine the
RID from the `rad` remote.
The name `print_inline` was used because, previously, the `print`
function name was taken.
This has now changed, allowing the rename of this function to happen.
The name `print` was used because, previously, the `println`
function name was taken.
This has now changed, allowing the rename of this function to happen.
Prevent `rad` from panicking when its stdout is a closed pipe, e.g.
when running `rad config | head`.
The broken pipes are handled at the output boundaries using three
layers of defence:
1. Inner: Route all stdout output through `term::print()` and
`term::print_inline()`, which use explicit `writeln!`/`write!` to
a locked stdout and silently ignore write errors. All `println!`
calls in `radicle-cli` and `radicle-term` are converted.
2. Middle: Catch `BrokenPipe` errors that propagate up via `anyhow`
from subcommands in `main::run()`, and exit cleanly with code 0.
3. Outer: Install a panic hook that intercepts `println!` panics
containing "Broken pipe" (from dependencies like clap) and exits
cleanly, chained in front of human_panic's hook.
To prevent regressions, `#![deny(clippy::print_stdout)]` is added to
`radicle-cli`, requiring all future stdout output to go through the
safe `term::` functions.
Note: `eprintln!` calls (stderr) are not handled, as broken stderr
pipes are extremely rare in practice.
Rust (since 1.62) ignores EPIPE by default (see [Rust #62569]),
causing writes to closed pipes to return `io::ErrorKind::BrokenPipe`
errors. The `println!` macro panics on these errors, producing a
confusing backtrace instead of the silent exit expected of Unix CLI
tools.
To avoid the use of `print!` and `println!`, the `radicle-term`
helper functions use `write!` and `writeln!`. This lays the groundwork
for the `rad` CLI to transition to using only `radicle-term`
functions.
There was a first attempt made, which is documented here. A
process-wide SIGPIPE reset (`SIG_DFL`) was ruled out because `rad`
manages child processes via libgit2 pipes and communicates with the
radicle node over Unix sockets. Resetting SIGPIPE globally caused
`rad` itself to be killed during internal pipe operations (e.g. during
`rad remote add --fetch`), producing flaky test failures.
[Rust #62569] https://github.com/rust-lang/rust/issues/62569
Add tests that verify the `rad` binary does not panic when its stdout
is a broken pipe (e.g., `rad config | head -1`).
Rust (since 1.62) ignores SIGPIPE by default (see [Rust #62569]),
causing `println!` to panic with "failed printing to stdout: Broken
pipe" instead of exiting silently. This is the standard Unix behaviour
expected of CLI tools.
These tests currently fail on `rad config` and `rad self`, and are set to `ignore`, to be enabled when fixed.
[Rust #62569]: https://github.com/rust-lang/rust/issues/62569
Many usages of `Oid::from_sha1` are actually in the context of
generating arbitrary `Oid`s. In these cases, `impl Arbitrary for Oid`
may be used directly.
The length of a SHA-1 object identifier is defined in various locations.
Instead of this redundancy, make the definition in `radicle-oid` public, and
use it in `radicle{-{core,protocol},}`.
A small refactor to move this constant closer to the `enum` that it is
about. An associated constant constant fits more nicely.
Reason for changing the name from `SHA1_*_LEN` to `LEN_SHA1`: As we
think about the object format in the future becoming variable, we
would have that the length of an OID is parameterized by the object
format. So, we imagine a function `fn len(format: ObjectFormat)`, to
be called as `Oid::len(ObjectFormat::Sha1)`. This is the new order
we use.
When the underlying hash function is not important, but only the fact
whether the OID is the zero sentinel, use `Oid::is_zero`. This is more
flexible, as it is forward compatible with the SHA-256 object format.
`fn sha1_zero` can be `const fn`, and since it does not take arguments,
it can just be a `const`. There is no good reason to not have a constant.
Justification for changing the order to first specify "zero" and then
"SHA1": In the future we will support multiple object formats. A
function that produces a zero OID would be parameterized by the object
format then. Imagine `fn zero(format: ObjectFormat)` which would be
called as `Oid::zero(ObjectForma::Sha1)`. In anticipation, we change the
ordering to match.
Because we changed our main domain from radicle.xyz to radicle.dev, we
should also change our systemd credential identifiers.
Handling is move into a new function, which also checks the old names
for backwards compatibility.
With `connect: std::collections::HashSet`, we suffer unpredictable reordering
of values in our test cases.
Use `connect: indexmap::IndexSet` instead, which preserves insertion
order for iteration.
Fixes an intermittent failure caused by a race condition with background
gossip messages.
Previously Eve's `RefsAnnounced` message could be processed by Bob
after the test updated the identity document to include Alice-Laptop
as a delegate. This caused Bob to unexpectedly fetch the issue in the
background, failing the `!has_issue` assertion.
We now explicitly wait for Bob to receive Eve's announcement before
updating the identity document.
Avoid relying on a thread sleep to wait for the nodes to connect.
Instead rely on the connection condition being established by using a polling loop.
The loop is controlled by a number of iterations, panicking if the
number of iterations is exceeded.
Addresses intermittent failures in the `test_connection_crossing` e2e test,
which were particularly prevalent on slower CI environments (such as
`rust:trixie`).
Previously, the test spawned two threads to make Alice and Bob dial each
other concurrently and strictly asserted that the "preferred" peer (the
one with the higher Node ID) would always win the `Outbound` link direction.
However, this assumption is flawed in real-world, OS-level network execution
due to two race conditions:
1. Thread Scheduling: One thread could execute and fully establish a
connection before the other thread even began processing its dial command.
2. Reactor Event Ordering: A node's reactor might wake up and process an
incoming TCP connection from its peer *before* it processes the `Connect`
command sent by the test. When it finally processes the `Connect` command,
it sees a session already exists and skips dialing entirely.
In both scenarios, a true "simultaneous crossing" never occurs. Instead, a
standard sequential connection happens, meaning the link direction is dictated
by whoever dialed first, not by the "preferred" peer logic. This caused the
strict `left: Outbound, right: Inbound` assertions to panic.
To fix this, two changes were made:
- Introduced a `std::sync::Barrier` to synchronize the two test threads.
This forces both threads to wait for each other before calling `.connect()`,
maximizing the probability of a true simultaneous dial.
- Relaxed the final assertions. Because OS-level TCP handshakes and reactor
polling can never guarantee perfect simultaneity, we no longer assert
*which* peer gets the `Outbound` link. Instead, we assert the core invariant:
that exactly one connection is established between the nodes, and that their
link directions are opposite (`s1.link != s2.link`).
Two tests assumed that `refs/remotes/rad/HEAD` would automatically be
crated. It turns out, however, that this only the case if executed
with Git 2.48.0 or newer. Git versions older than 2.48.0 do not touch
'refs/remotes/<remote>/HEAD'.
Git 2.48.0 and newer will update `refs/remotes/<remote>/HEAD` by
default. This can be disabled by setting
`remote.<remote>.followRemoteHEAD` to "never". Thus, with these
versions, setting that configuration to "never" emulates the behaviour
of older versions.
Therefore, to ensure that test results are consistent across Git
versions before and after 2.48.0, the fixture of test repositories
now include setting 'remote.rad.followRemoteHEAD = never'.
The affected tests are adjusted accordingly.
This change can possibly be reverted once Radicle requires usage of Git
2.48.0 or newer.
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.xyz>
Multiple commands take an RID as a command line argument. However, the
handling, which usually involves falling back to detection of the
repository associated with the current directory, is duplicated many
times over.
Introduce `fn rid_or_cwd` which can be used by many commands.
Many commands take RIDs as arguments. Most of them call the
corresponding struct member `repo`, some call it `rid`.
For consistency, name these members `repo` for all commands.
This change is intended to be invisible to the user. That is why,
in some cases, the "long" version of the argument must be renamed to
match `--rid`.
The main change that needs adjustment on our side is that
`impl EcPk for ec25519::PublicKey` changed from `Compressed = [u8; 32]`
to `Compressed = amplify::Bytes32`.
Also remove `impl Deref for PublicKey` to avoid dependents of
`radicle-crypto` depend on a particular implementation, and make the
anonymous member private.
See
<f42396139a>.
`radicle-cli-test` initiates `cargo` builds under the assumption
that the target directory is always within the source directory.
However, the user may have a different idea, specified with the
environment variable `CARGO_TARGET_DIR` (supported by `cargo`, see
https://doc.rust-lang.org/cargo/reference/environment-variables.html).
Add support for `CARGO_TARGET_DIR`, with a fallback to 'target'
at the top of the source tree if `CARGO_TARGET_DIR` is undefined.
The value of `RAD_SOCKET` influences the outcome of `cargo test`.
Users that set this environment variable to, e.g. point `rad` at
a socket at a custom location, were surprised that execution of
tests would interact with their node.
Split `Home::socket` into two functions. `Home::socket_default` is
"pure" and only dependent on the path represented by `Home`. On the
other hand, `Home::socket_from_env` interprets the environment.
Most users will want to use `Home::socket_from_env`.
There is only one type that can really construct canonical references,
which is the the identity document. Remove the corresponding method
from the trait and rename it to `GetRawCanonicalRefs` accordingly.
Separate module for the concern of protecting `refs/rad`.
Note that this also fixes a bug, as previously all refs
*starting with* `refs/rad`, such as `refs/radieschen` were protected.
The method `set_head` does two things:
1. Compute the canonical head and set the default branch to target.
2. Set the symbolic reference `HEAD` to target the default branch.
Split these two concerns into:
1. `set_default_branch_to_canonical_head`
2. `set_head_to_default_branch`
Lorenz Leutgeb
Fintan Halpenny
stefan
Adrian Duke
Daniel Norman
Lorenz Leutgeb
Josh Soref
:raw`
Richard Levitte