aws4fetch icon indicating copy to clipboard operation
aws4fetch copied to clipboard

x-forwarded-for, cf-connecting-ip, etc. causes failed signature

Open timkelty opened this issue 4 months ago • 4 comments

In my Cloudflare worker, signing fails unless I remove these headers first:

		'cf-connecting-ip',
		'cf-ipcountry',
		'cf-ray',
		'x-forwarded-for',
		'x-real-ip',

In my case, I'm working around like this:

	// Signing will fail if these headers are present 🤷
	const unsignableHeaders = [
		'cf-connecting-ip',
		'cf-ipcountry',
		'cf-ray',
		'x-forwarded-for',
		'x-real-ip',
	];

	unsignableHeaders.forEach((name) => {
		console.log("Removing unsignable header", name);
		originRequest.headers.delete(name);
	});

	return aws.sign(originRequest);

Perhaps there should be an option to pass unsignableHeaders as an option?

timkelty avatar Sep 04 '25 11:09 timkelty

Sounds like you're proxying a request? Normally you'd be constructing a request from scratch to call an API. What's the use case here out of interest?

(just to set expectations – it's unlikely I'll add this as an option here as it's pretty easy to just do yourself)

mhart avatar Sep 04 '25 22:09 mhart

Yep! In this case it's: Cloudflare Worker (suppling Request) → AWS Lambda URL. The entire request is proxied, with the host swapped out.

What I have works (and I don't need those headers), but I'm concerned about other headers that might be a problem in the future.

If I essentially need access to signableHeaders – would you suggest I instead use AwsV4Signer, and set that after instantiation?

If you were up for a PR, a couple of ideas:

Non-breaking (but a little awkward)

  • Change allHeaders to be boolean | object | Headers – boolean would be current behavior, the others would pass to signableHeaders.

Breaking

  • Deprecate allHeaders in favor of something like signableHeaders that can take an *.

Both of these would benefit from exporting the UNSIGNABLE_HEADERS map you have.

I'd be happy to PR either or if you had something else in mind, just let me know what you're thinking.


Thanks for the quick response. Loving the library – I was annoyed when I saw I had to convert everything to HttpRequest, so was grateful to find this!

timkelty avatar Sep 05 '25 11:09 timkelty

would you suggest I instead use AwsV4Signer, and set that after instantiation?

That doesn't really work, as other things happen in the constructor that rely on that.

Here's what I'm doing instead – only signing a subset, then merging them all back after signing:

export const signRequest = async (
	request: Request,
	aws: AwsClient,
): Promise<Request> => {
	const signableHeaders = [
		"host",
		"accept",
		"content-type",
		"content-length",
		"cookie",
		"user-agent",
	];

	const headersToSign = Object.fromEntries(
		[...request.headers.entries()].filter(([key]) =>
			signableHeaders.includes(key.toLowerCase()),
		),
	);

	const signedRequest = await aws.sign(request, {
		headers: headersToSign,
	});

	return new Request(signedRequest, {
		headers: Object.assign(
			Object.fromEntries(request.headers.entries()),
			Object.fromEntries(signedRequest.headers.entries()),
		),
	});
};

timkelty avatar Sep 05 '25 18:09 timkelty

Note: Cloudflare may also alter the accept-encoding header. This random failure took me so long to figure out.

Func86 avatar Oct 23 '25 09:10 Func86