Cache encoding/layout incompatibility causes request failure

[woodruffw]

Hi there!

I was experimenting with upgrading zizmor to reqwest 0.13, which includes upgrading http-cache-reqwest and reqwest-middleware for compatibility reasons.

Expected behavior

Invalid or incompatible (e.g. for legacy reasons) cache entries should be silently ignored (potentially with logging), rather causing a hard failure that takes down the entire request.

Actual behavior

Everything built fine and appears to work on a machine with no pre-existing cache (or only fresh cache entries). However, if I perform an invocation that hits the network on a machine that does have a pre-existing cache, I get this error:

% GH_TOKEN=$(gh auth token) ./target/debug/zizmor .github/workflows/benchmark.yml
🌈 zizmor v1.22.0
fatal: no audit was performed
'known-vulnerable-actions' audit failed on file://.github/workflows/benchmark.yml

Caused by:
    0: error in 'known-vulnerable-actions' audit
    1: request error while accessing GitHub API
    2: Cache error: invalid value: integer `28`, expected variant index 0 <= i < 1

If I blow away the cache (at ~/.cache/zizmor) it disappears, although I can make it reappear reliably by running a previous release, presumably creating the old (incompatible) entries again:

% GH_TOKEN=$(gh auth token) zizmor .github/workflows/benchmark.yml
🌈 zizmor v1.22.0
 INFO audit: zizmor: 🌈 completed .github/workflows/benchmark.yml
No findings to report. Good job!

...and then if I run GH_TOKEN=$(gh auth token) ./target/debug/zizmor .github/workflows/benchmark.yml, the error reappears.

Conversely, if I blow away the cache and run the new version first, the old version also fails, but with a different (less detailed) error:

% GH_TOKEN=$(gh auth token) zizmor .github/workflows/benchmark.yml
🌈 zizmor v1.22.0
fatal: no audit was performed
'known-vulnerable-actions' audit failed on file://.github/workflows/benchmark.yml

Caused by:
    0: error in 'known-vulnerable-actions' audit
    1: request error while accessing GitHub API
    2: Cache error: io error:

The previous (currently released) version is using http-cache-reqwest 1.0.0-alpha.2; the new (unreleased) version is using http-cache-reqwest 1.0.0-alpha.3.

I'm pretty sure this is an error within http-cache, and not within another part of reqwest or reqwest-middleware. In particular, this looks like a likely source:

http-cache/http-cache/src/lib.rs

Lines 591 to 646 in a400966

	// Serialize as enum (needed for bincode to match deserialization)
	impl Serialize for HttpHeaders {
	fn serialize<S>(
	&self,
	serializer: S,
	) -> std::result::Result<S::Ok, S::Error>
	where
	S: Serializer,
	{
	// Serialize the enum properly with variant tags
	// This matches how RawHttpHeaders deserializes
	match self {
	HttpHeaders::Modern(modern) => serializer
	.serialize_newtype_variant("HttpHeaders", 0, "Modern", modern),
	#[cfg(feature = "http-headers-compat")]
	HttpHeaders::Legacy(legacy) => serializer
	.serialize_newtype_variant("HttpHeaders", 1, "Legacy", legacy),
	}
	}
	}
	// Deserialize: Need to handle enum variant properly for bincode
	impl<'de> Deserialize<'de> for HttpHeaders {
	fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
	where
	D: Deserializer<'de>,
	{
	// Bincode serializes enums with a variant index
	// We need to deserialize it properly as an enum
	#[cfg(feature = "http-headers-compat")]
	{
	#[derive(Deserialize)]
	enum RawHttpHeaders {
	Modern(HashMap<String, Vec<String>>),
	Legacy(HashMap<String, String>),
	}
	match RawHttpHeaders::deserialize(deserializer)? {
	RawHttpHeaders::Modern(m) => Ok(HttpHeaders::Modern(m)),
	RawHttpHeaders::Legacy(l) => Ok(HttpHeaders::Legacy(l)),
	}
	}
	#[cfg(not(feature = "http-headers-compat"))]
	{
	#[derive(Deserialize)]
	enum RawHttpHeaders {
	Modern(HashMap<String, Vec<String>>),
	}
	match RawHttpHeaders::deserialize(deserializer)? {
	RawHttpHeaders::Modern(m) => Ok(HttpHeaders::Modern(m)),
	}
	}
	}
	}

(I noticed the http-headers-compat feature, but that seems to only be exposed on http-cache and not http-cache-reqwest.)

[06chaynes]

Thanks for the detailed report! I'll check into this and let you know what I find.

[06chaynes]

Ok I believe I have located several mistakes I made in that last release, I went ahead and yanked it while I address this.

I need to expose the missing flags, I removed #[serde(untagged)] on the new enum and forgot to fix that, AND I needed to derive Default on the new metadata field.

[woodruffw]

Thanks a ton for the fast response!

[06chaynes]

I am still testing but I just pushed up some changes that I think resolves this. Adding the http-headers-compat feature should do it for now at least. I still need better handling for this

Edit: Think I still have something to work out, I will look into this further tomorrow!

Oh You also need the manager-cacache-bincode feature

Due to the bincode deprecation postcard is now the default

[06chaynes]

I thought I could tag the metadata field with Default and that would work but it was throwing errors, so when using the manager-cacache-bincode metadata is always set to None. I tested on a cache from an older version and it deserialized fine so that combo should be good. I will try to cut a new release soon but I want to do some more testing and thinking on this path forward.

[woodruffw]

Thanks again @06chaynes! I tried bumping to the latest alpha and made a bit more progress, but I'm seeing another error get surfaced:

🌈 zizmor v1.22.0
fatal: no audit was performed
error: configuration error in .github/zizmor.yml

Caused by:
    0: configuration error in .github/zizmor.yml
    1: GitHub API error while fetching remote config
    2: request error while accessing GitHub API
    3: Cache error: Tried to parse invalid utf-8

From a quick look, this is probably the same as #146 🙂

(That's after priming the cache with an earlier version.)

Edit: whoops, realized I didn't set the newly exposed feature. I'll try that.

[woodruffw]

Hmm, no luck with those features unfortunately. This is what I have:

http-cache-reqwest = { version = "1.0.0-alpha.4", features = ["manager-moka", "http-headers-compat", "manager-cacache-bincode"] }

[06chaynes]

Forgot to update but I am looking into this, thanks again for the testing!

[woodruffw]

Sorry for another ping on this -- do you have a rough sense of when another alpha will be out that I can test against here?

(No worries at all if it's a while! I just want to plan out a workaround for zizmorcore/zizmor#1553 in that case. Thanks, as always, for your maintenance efforts!)

[06chaynes]

Sorry, I got everything in place then got lost in other tasks. I think the changes here #154 resolve this. I added a lot more tests around this problem specifically and it seems to be good now. Are you able to test from that branch by chance or should I go ahead and cut a new release (no problem if not).

The readme in the tests directory goes over the three logical paths available: https://github.com/06chaynes/http-cache/blob/ec20bbce133dbd3d50eff6309d18c511f33d38ac/compat-tests/README.md

[woodruffw]

Thanks a ton! And no worries at all, I appreciate your time.

Are you able to test from that branch by chance or should I go ahead and cut a new release (no problem if not).

I should be able to be able to give it a test tonight from your branch!

[woodruffw]

Thanks @06chaynes, I can confirm that branch works for me! This is what I did:

http-cache-reqwest = { version = "1.0.0-alpha.4", features = ["manager-moka"], git = "https://github.com/06chaynes/http-cache", branch = "compat-tests" }

It emits a lot of warnings, though, e.g.:

 WARN collect_inputs:fetch_single_file: http_cache::managers::cacache: Failed to deserialize cache entry for key 'GET:https://api.github.com/repos/astral-sh/uv/contents/.github/zizmor.yml': Tried to parse invalid utf-8
 WARN collect_inputs:fetch_single_file: http_cache::managers::cacache: Failed to deserialize cache entry for key 'GET:https://api.github.com/repos/astral-sh/uv/contents/.github/zizmor.yaml': Tried to parse invalid utf-8
 WARN collect_inputs:fetch_single_file: http_cache::managers::cacache: Failed to deserialize cache entry for key 'GET:https://api.github.com/repos/astral-sh/uv/contents/zizmor.yml': Tried to parse invalid utf-8
 WARN collect_inputs:fetch_single_file: http_cache::managers::cacache: Failed to deserialize cache entry for key 'GET:https://api.github.com/repos/astral-sh/uv/contents/zizmor.yaml': Tried to parse invalid utf-8

Would it be possible to get those lowered to DEBUG or TRACE? I think that'd be ideal for a piece of middleware 🙂

[06chaynes]

Ah yeah good catch, I will tweak that and cut a new release later today. Thanks again for your feedback!