fix(ic-asset): .ic-assets.json allow_raw_access overrides

[smallstepman]

Description

.ic-assets.json: "allow_raw_access" default overrides previous config entries even if not present in a match entry. This fixes the issue.

Jira ticket description

Consider the following .ic-assets.json file.

[
	{
		"match": "**/favicons/**/*",
		"allow_raw_access": true
	},
	{
		"match": ".well-known",
		"ignore": false
	},
	{
		"match": "**/*",
		"headers": {
			"X-Frame-Options": "DENY",
			"X-Content-Type-Options": "nosniff",
			"Strict-Transport-Security": "max-age=31536000; includeSubDomains",
			"Referrer-Policy": "same-origin",
			"X-XSS-Protection": "1; mode=block"
		}
	}
]

If there is a file in the favicons/ directory, we would expect its allow_raw_access setting to be true.

However, the "match": "**/*" section also matches for files in the favicons/ directory. Even though the configuration does not specify a value for allow_raw_access, a default of false is assumed.

So given the above configuration, the actual property seen for allow_raw_access is false.

Note: the default value of allow_raw_access has changed from false to true in dfx 0.14.4 and later, so the problem won’t present exactly the same.

A fix is to reorder the configuration, to put the favicons directory match at the end:

[
	{
		"match": ".well-known",
		"ignore": false
	},
	{
		"match": "**/*",
		"headers": {
			"X-Frame-Options": "DENY",
			"X-Content-Type-Options": "nosniff",
			"Strict-Transport-Security": "max-age=31536000; includeSubDomains",
			"Referrer-Policy": "same-origin",
			"X-XSS-Protection": "1; mode=block"
		}
	},
	{
		"match": "**/favicons/**/*",
		"allow_raw_access": true
	},
]

However, note that there could be similar problems with other default settings.

Expected behavior: No matter the order of the above configuration, files in the favicons/ directory (in this example) should have allow_raw_access true. Since the "match": "**/*" configuration section does not contain allow_raw_access, it should not affect the configuration of that value.

Fixes https://dfinity.atlassian.net/browse/SDK-1238

How Has This Been Tested?

unit test

Checklist:

• The title of this PR complies with Conventional Commits.
• I have edited the CHANGELOG accordingly.
• I have made corresponding changes to the documentation.

[smallstepman]

incorrectly introduced in #3369

[smallstepman]

lint

[smallstepman]

makes life easier during debugging

[ericswanson-dfinity]

Would you please write something in the PR description about how this PR fixes the issue? I see changes to use the derivative crate, but that looks like a refactor, not something that changes functionality.

Also please describe the problem that this solves in a little more detail in the PR description, since access to JIRA is not public.

[smallstepman]

To be perfectly honest, I don't know why it fixes it, but it fixes it, took me a long time to find it simply because I stumbled upon it by means of trial and error. I tried using #[serde(default = "...")] all over the place (instead of derivative), but it caused more issues; somehow using derivative fixes the issue

[smallstepman]

I pasted the content of jira ticket into pr description

[ericswanson-dfinity]

Not to be merged until we understand how/why this PR fixes the issue.

[ericswanson-dfinity]

In the changelog, please describe the issue in enough detail that someone with a project, the behavior of which will change after this fix, will be able to recognize that fact.

[ericswanson-dfinity]

I pasted the content of jira ticket into pr description

I appreciate the effort, but unfortunately it's not sufficient in this case.

The JIRA ticket describes a problem. Its purpose is to describe that problem in enough detail that the assignee can understand what's being asked for, and hopefully reproduce the problem. It doesn't describe the cause of the problem because it can't: the cause isn't known at the time of writing of the JIRA ticket.

The PR description should describe the change and the reasoning behind it. The PR author should understand their change well enough to describe what they did and why. The PR description should give reviewers an idea of what they are looking at. A succinct yet complete PR description will also demonstrate to reviewers that the PR author understands their own change.

[ericswanson-dfinity]

An idea as to the underlying cause: this removes the serde default, replacing it with an "impl Default" default. So before, whereas deserializing a rule that didn't specify allow_raw_access would have Some(true) here, after this change it will have None.

What would you think about removing the #[derivative(Default(value = "Some(true)"))] as well? At this level, we're interested in knowing what's in the config file rule, and None seems more accurate.

[sa-github-api]

Dear @smallstepman,

In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA.

If you decide to agree with it, please visit this issue and read the instructions there. Once you have signed it, re-trigger the workflow on this PR to see if your code can be merged.

— The DFINITY Foundation

[ericswanson-dfinity]

Replaced by #3635