fix: Sanitize manifest property values before interpolation

[Rob--W]

Fixes #2119 (which was a regression by #1900).

[rpl]

lgtm, thanks Rob.
I just have one question, but it should affect the fix applied in this PR and so I'm also approving this PR as is.

[rpl]

Nit/question: why putting .zip is the description? it is definitely expected that ".zip" ends up into the generated filename, but I was wondering what is the role of it in the test (because it is not immediately clear what those that test, may be good to mention why we have included it in the description as part of the inline comment at line 130 in test.build.js, which is currently mentioning that .zip is coming from the description but not why we are doing it in that test).

[Rob--W]

It's not significant, and unrelated to the fixed issue. I just put it there so that there is test coverage for the scenario where the extension is derived from the interpolated value. That would catch the case if someone ever tried to enforce the validation on the template (instead of the parsed result).

I ended up fixing the issue by sanitizing the interpolated values, but an alternative/additional change that I considered was to move the path check to before the interpolation (since it's logically equivalent now)

[rpl]

That would catch the case if someone ever tried to enforce the validation on the template (instead of the parsed result).

I see, then I think that is something that would be good to be pointed out in that inline comment in at test.build.js line 130, because that comment does explicitly say that we expect it but not that it is intended to catch a possible regression.

[Rob--W]

I just found that I had non-commited changes with the following comment:

diff --git a/tests/unit/test-cmd/test.build.js b/tests/unit/test-cmd/test.build.js
index 04745db..cc12d48 100644
--- a/tests/unit/test-cmd/test.build.js
+++ b/tests/unit/test-cmd/test.build.js
@@ -128,6 +128,9 @@ describe('build', () => {
           sourceDir: fixturePath('slashed-name'),
           artifactsDir: tmpDir.path(),
           // name contains a slash. description ends with ".zip".
+          // This allows us to verify that the requirement (i.e. file name
+          // should end with a zip or xpi extension) is enforced for the
+          // resulting filename and not just the template.
           filename: 'prefix-{name}{description}',
         })
           .then((buildResult) => {

This is just a test, so I don't really care that much about the inline comment, but if you consider it important I could submit a PR with the comment fix.