sqlite tests failing in Fedora packaging environment

[LecrisUT]

I am lost on this issue. When running the tests, I get the following error:

---- database::test::test_search_bench_dupes stdout ----
thread 'database::test::test_search_bench_dupes' panicked at atuin-client/src/database.rs:1069:18:
called `Result::unwrap()` on an `Err` value: Database(SqliteError { code: 1, message: "no such table: history" })

Running locally I am not able to reproduce this error.

I have created a mock environment (equivalent to a docker container but for package builds):

Mock environment

`~/.config/mock/atuin.cfg` ```cfg include('/etc/mock/fedora-rawhide-x86_64.cfg') config_opts['root'] = 'fedora-rawhide-atuin' config_opts[f'{config_opts.package_manager}.conf'] += """ [atuin] name=Atuin copr baseurl=https://copr-be.cloud.fedoraproject.org/results/packit/SriRamanujam-atuin-rpmspec-7/fedora-rawhide-$basearch/ type=rpm-md skip_if_unavailable=False gpgcheck=1 gpgkey=https://copr-be.cloud.fedoraproject.org/results/packit/SriRamanujam-atuin-rpmspec-7/pubkey.gpg repo_gpgcheck=0 enabled=1 enabled_metadata=1 priority=9 [paserk_paseto] name=Paserk paseto copr baseurl=https://copr-be.cloud.fedoraproject.org/results/packit/LecrisUT-rusty_parserk-paseto-rpmspec-main/fedora-rawhide-$basearch/ type=rpm-md skip_if_unavailable=False gpgcheck=1 gpgkey=https://copr-be.cloud.fedoraproject.org/results/packit/LecrisUT-rusty_parserk-paseto-rpmspec-main/pubkey.gpg repo_gpgcheck=0 enabled=1 enabled_metadata=1 priority=9 [metrics] name=Metrics copr baseurl=https://copr-be.cloud.fedoraproject.org/results/packit/LecrisUT-rust-metris-rpmspec-main/fedora-rawhide-$basearch/ type=rpm-md skip_if_unavailable=False gpgcheck=1 gpgkey=https://copr-be.cloud.fedoraproject.org/results/packit/LecrisUT-rust-metris-rpmspec-main/pubkey.gpg repo_gpgcheck=0 enabled=1 enabled_metadata=1 priority=9 [sqlx] name=Sqlx copr baseurl=https://copr-be.cloud.fedoraproject.org/results/packit/LecrisUT-sqlx-rpmspec-main/fedora-rawhide-$basearch/ type=rpm-md skip_if_unavailable=False gpgcheck=1 gpgkey=https://copr-be.cloud.fedoraproject.org/results/packit/LecrisUT-sqlx-rpmspec-main/pubkey.gpg repo_gpgcheck=0 enabled=1 enabled_metadata=1 priority=9 """ ``` And running the following to get into the build environment (don't know how reproducible this is in other environments) ```console $ wget https://download.copr.fedorainfracloud.org/results/packit/SriRamanujam-atuin-rpmspec-7/fedora-rawhide-x86_64/07513006-atuin/atuin-18.2.0-1.fc41.src.rpm ^Downloads the srpm to get the source and try build atuin^ $ mock -r atuin ./atuin-18.2.0-1.fc41.src.rpm ^Runs the build process^ $ mock -r atuin --install emacs less ^Install some tools you may need^ $ mock -r atuin --shell ^Enter the build environment^ ^Next is just some setups to mimic the build environment^ # cd /builddir/build/BUILD/atuin-18.2.0-build/atuin-18.2.0 # export CARGO_HOME=/builddir/build/BUILD/atuin-18.2.0-build/atuin-18.2.0/.cargo # export RUSTFLAGS="-Copt-level=3 -Cdebuginfo=2 -Ccodegen-units=1 -Cstrip=none -Cforce-frame-pointers=yes -Clink-arg=-specs=/usr/lib/rpm/redhat/redhat-package-notes --cap-lints=warn" # export RUSTC_BOOTSTRAP=1 # export RPM_PACKAGE_NAME=atuin # export RPM_PACKAGE_VERSION=18.2.0 # export RPM_PACKAGE_RELEASE=1 ^Next I just run a random test^ # cargo test -Z avoid-dev-deps --profile rpm --package atuin-client --lib database::test::test_search_fuzzy -- --exact ```

From all of my experimentation with debug vs --profile rpm, removing CARGO_HOME and RUSTFLAGS, removing -Z avoid-dev-deps, and such, I couldn't find something in there that caused such issues.

I have tried editing atuin-client/migrations to make it break, and indeed it showed that the migration was actually run, so I am not sure what else to try for this issue.

Edit: Narrowed it down to sqlite::memory:, changing it to some dummy file works.

[xvello]

Heya @LecrisUT,

Looking at your packaging work, it looks like you are building Atuin against a patched sqlx crate to force a different libsqlite3-sys version, built to dynamically link against the system's sqlite lib, that is different from upstream's tested version, and variable between distros. The tests failing prove that this approach has a high risk of introducing bugs, up to and including data corruption / data loss.

C / C++ codebases are usually written to work around dependency drift (by checking lib versions at runtime and branching into different code paths), but Rust and Golang codebases assume that dependencies are locked, so dependency drift will break invariants in subtle ways.

I understand that you are following Fedora's outdated packaging policy, but know that openSUSE moved away from this and is now recommending vendoring deps as locked by upstream. It is up to Ellie to decide whether to support what is effectively a fork distributed with the same name, but I would refuse to do so on projects I maintain.

Until the Fedora packaging policies evolve, my recommendation would be to distribute rpms and debs via openSUSE's Build Service, as I described in this comment. They do support building and publishing packages for openSUSE, Fedora, Debian & Ubuntu, on several architectures. This way, we can distribute builds that use the deps that project maintainers have thoroughly tested and can support. I'd be happy to help add deb packaging, would you want to kick the tires on the rpm side?

[LecrisUT]

I was looking at the commit that bumped libsqlite3-sys ^1 but that didn't show any changes in the api, but maybe it was fixed at a later time.

The issue in this failure is that system sqlite was never tested, instead only the vendored version was used. In my tests, the actual file-system sqlite passed the tests

I understand that you are following Fedora's outdated packaging policy

Each method has its pros and cons. In this case it is easier to catch and propagate CVE fixes and it is more suited for multi-language packages, libsqlite3-sys being one of them. Another benefit is that we can find issues earlier and contribute upstream more easily.

But of course that makes packaging process quite a mess, and I am not suggesting that upstream to get involved in maintaining this. There are more suitable options (see next section). But I still believe it's beneficial to have some distros pursuit such packaging models because it can help propagate new projects fixes, etc.

This way, we can distribute builds that use the deps that project maintainers have thoroughly tested and can support

For upstream developers I would agree that they should provide a flatpak (wouldn't work for this case unfortunately) and vendored version, and there is a version on copr ^2 using that model. I don't know about the upstream support of open suse's infrastructure, but for copr we have packit that we can deploy for upstream if they want it.

I'd be happy to help add deb packaging, would you want to kick the tires on the rpm side?

Feel free to use the version in ^2. The latest PR has all of the tests enabled and I'll add the bash-pre-exec dependency soon so that it's functional out-of-the-box.

[ellie]

I'm afraid that unless you can reproduce this failure by

• introducing another test to our codebase that demonstrates the failure
• reproducing this on a build of Atuin that has no patches applied, and uses our lockfile

Then this issue does not belong on our repo, and we will not be able to dedicate any time to supporting it

For upstream developers I would agree that they should provide a flatpak

We provide statically-linked binaries built for a few different platforms and libc implementations - we will not be providing a Flatpak, Snap, AppImage, etc any time soon.

Otherwise, you may find this discussion with the Void package maintainer insightful: https://forum.atuin.sh/t/packaging-atuin-for-void-linux/20

They use a SQLite shared library, however all other dependencies are those specified in our Cargo.lock. The issue they had was with some build flags on their version of SQLite.

[LecrisUT]

Then this issue does not belong on our repo, and we will not be able to dedicate any time to supporting it

Indeed, this would either be on sqlx-sqlite or libsqlite3-sys side (the latter is tested but I don't know how thorough). I think the only way to actually catch such issues is if in the github workflows a build environment using system bindings can be used forcing buildtime_bindgen feature?

I'll keep a note on the references you've shared of Void and try to investigate around that. Thanks for that tip.

[ellie]

I think the only way to actually catch such issues is if in the github workflows a build environment using system bindings can be used forcing buildtime_bindgen feature?

We won't be supporting builds that decide to use system bindings, if package repositories choose to do so then that is their decision

[LecrisUT]

We won't be supporting builds that decide to use system bindings, if package repositories choose to do so then that is their decision

Sure, that is perfectly reasonable.

About the vendored version, we have https://github.com/SriRamanujam/atuin-rpmspec, and there is an integration with packit to automatically make test builds at each commit/PR/release. This could be a more upstream official recommended distribution, with the downside that upstream will now be responsible for CVE and rebuilds (there is no mechanism to specify a copr patch is security fix vs feature vs bug fix for example). Nevertheless, I would be more than happy to help implement and discuss this approach. An alternative would be open-build-system that @xvello suggested, but I am not familiar with the automation and user experience.

[xvello]

about CVEs: I don't think any sqlite CVE would be relevant to Atuin's current threat model:

• the server uses PG for storage and not sqlite
• IIRC there's an un-official build of the server that uses sqlite for storage, but it's not officially supported yet?
• atuin-client cannot not be used to escalate privileges outside of the owning user, so a CVE there would not have a significant impact
• atuin-daemon (optional, disabled by default) could theoretically be used to attack another local user's data by bypassing the socket's unix permissions. I recommend implementing SO_PEERCRED (check that the peer process has the same UID as the daemon, reject requests if not) for a second layer of defense

[LecrisUT]

CVEs here are any CVEs of any rust packages that are in vendor. Who knows what does might consist of. On the distro side (without vendoring), that is actively monitored by the packagers. On the vendored case (or distro side that use vendoring) it would be upstream's responsibility to monitor these. Probably dependabot would pick up on necessary CVEs, although, if something is changed in a dependency chain (maybe on a dependency's patch version in a second layer (not in atuin's Cargo.toml)), it might not be updated until the next rebuild.

[ellie]

On the vendored case (or distro side that use vendoring) it would be upstream's responsibility to monitor these.

It is always our responsibility to monitor our software for CVEs. There are many, many places other than Fedora where users can download Atuin, so we must fix the CVEs we are affected by promptly. The suggestion that we'd leave security patching up to downstream package maintainers is unrealistic.

Probably dependabot would pick up on necessary CVEs, although, if something is changed in a dependency chain (maybe on a dependency's patch version in a second layer (not in atuin's Cargo.toml))

This is all caught by the lockfile, which contains both direct and transitive dependencies. If builds use the lockfile, then we can confirm whether they are affected by CVEs.

[LecrisUT]

This is all caught by the lockfile, which contains both direct and transitive dependencies. If builds use the lockfile, then we can confirm whether they are affected by CVEs.

Ok that's reassuring

The suggestion that we'd leave security patching up to downstream package maintainers is unrealistic.

Hmm, then what are your thoughts on the Fedora packaging? The guidelines are that the packages are not to be vendored, and it is the rust-SIG responsibility to follow CVEs and make appropriate patches because they are altering the lockfile and they need to propagate rebuilds due to lower-level (non-rust) CVEs as well. A complicated example is if a vulnerability is in a static-like library (let's say in a C++ library using templates) that is used by a rust dependency, than something like copr package with vendored dependencies would not be able to pickup the rebuild that is done on the OS until the next rebuild.

Would also support the decision to have only vendored version of the project with upstream having as much control of the distribution as possible, if that is the preferred management level. I could also ask a senior Fedora rust packager to pitch in if you would like more authoritative and experienced voices from the Fedora community. I may well be under/over-complicating things.