From 0bd2f0df17c588d8de9db76b701a9ac48fd379af Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 13 Feb 2025 07:47:53 -0800 Subject: [PATCH 1/3] Misc output changes --- e2e/tests-dfx/assetscanister.bash | 2 +- src/canisters/frontend/ic-asset/src/sync.rs | 11 ++++++---- src/dfx/src/commands/deploy.rs | 11 +++++----- src/dfx/src/lib/builders/assets.rs | 23 ++++++++++++++++++--- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/e2e/tests-dfx/assetscanister.bash b/e2e/tests-dfx/assetscanister.bash index c4dfb79bea..9aa103a641 100644 --- a/e2e/tests-dfx/assetscanister.bash +++ b/e2e/tests-dfx/assetscanister.bash @@ -1444,7 +1444,7 @@ EOF assert_command dfx deploy assert_contains "This project uses the default security policy for some assets." - assert_contains "Unhardened assets: all" + assert_not_contains "Unhardened assets:" assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_match "content-security-policy" assert_match "permissions-policy" diff --git a/src/canisters/frontend/ic-asset/src/sync.rs b/src/canisters/frontend/ic-asset/src/sync.rs index b17379e4ad..d1453f8a86 100644 --- a/src/canisters/frontend/ic-asset/src/sync.rs +++ b/src/canisters/frontend/ic-asset/src/sync.rs @@ -413,11 +413,14 @@ pub(crate) fn gather_asset_descriptors( .filter(|asset| asset.config.warn_about_standard_security_policy()) .collect_vec(); if !standard_policy_assets.is_empty() { - warn!(logger, "This project uses the default security policy for some assets. While it is set up to work with many applications, it is recommended to further harden the policy to increase security against attacks like XSS."); - warn!(logger, "To get started, have a look at 'dfx info canister-security-policy'. It shows the default security policy along with suggestions on how to improve it."); - if standard_policy_assets.len() == asset_descriptors.len() { - warn!(logger, "Unhardened assets: all"); + let qnt = if standard_policy_assets.len() == asset_descriptors.len() { + "all" } else { + "some" + }; + warn!(logger, "This project uses the default security policy for {qnt} assets. While it is set up to work with many applications, it is recommended to further harden the policy to increase security against attacks like XSS."); + warn!(logger, "To get started, have a look at 'dfx info canister-security-policy'. It shows the default security policy along with suggestions on how to improve it."); + if standard_policy_assets.len() != asset_descriptors.len() { warn!(logger, "Unhardened assets:"); for asset in &standard_policy_assets { warn!(logger, " - {}", asset.key); diff --git a/src/dfx/src/commands/deploy.rs b/src/dfx/src/commands/deploy.rs index ffc0ef32c9..0451334eb3 100644 --- a/src/dfx/src/commands/deploy.rs +++ b/src/dfx/src/commands/deploy.rs @@ -238,13 +238,14 @@ fn display_urls(env: &dyn Environment) -> DfxResult { let green = Style::new().green(); if !frontend_urls.is_empty() { info!(log, " Frontend canister via browser:"); - for (name, (url1, url2)) in frontend_urls { - if let Some(url2) = url2 { + for (name, (query_url, subdomain_url)) in frontend_urls { + if let Some(subdomain_url) = subdomain_url { info!(log, " {}:", name); - info!(log, " - {}", green.apply_to(url1)); - info!(log, " - {}", green.apply_to(url2)); + #[rustfmt::skip] + info!(log, " - {} (Recommended)", green.apply_to(subdomain_url)); + info!(log, " - {} (Legacy)", green.apply_to(query_url)); } else { - info!(log, " {}: {}", name, green.apply_to(url1)); + info!(log, " {}: {}", name, green.apply_to(query_url)); } } } diff --git a/src/dfx/src/lib/builders/assets.rs b/src/dfx/src/lib/builders/assets.rs index 03225bfa9c..4c2c59bcd5 100644 --- a/src/dfx/src/lib/builders/assets.rs +++ b/src/dfx/src/lib/builders/assets.rs @@ -14,8 +14,10 @@ use candid::Principal as CanisterId; use console::style; use dfx_core::config::model::network_descriptor::NetworkDescriptor; use fn_error_context::context; +use itertools::Itertools; use slog::{debug, info, o, Logger}; use std::fs; +use std::io::ErrorKind; use std::path::Path; use std::process::{Command, Stdio}; @@ -232,9 +234,24 @@ fn build_frontend( .stderr(Stdio::piped()); debug!(logger, "Running {cmd:?}..."); - let output = cmd - .output() - .with_context(|| format!("Error executing {cmd:#?}"))?; + let res = cmd.output(); + let output = match res { + Ok(o) => o, + Err(e) => { + let root_err = if e.kind() == ErrorKind::NotFound { + anyhow!("npm was not found. (Is it installed?)") + } else { + e.into() + }; + return Err(root_err).context(format!( + "Error executing \"{}\" {}", + shell_words::quote(&cmd.get_program().to_string_lossy()), + cmd.get_args() + .map(|a| shell_words::quote(&a.to_string_lossy()).into_owned()) + .format(" ") + )); + } + }; if !output.status.success() { return Err(DfxError::new(BuildError::CommandError( format!("{cmd:?}",), From 32905a5448a4530b878fd32befeb1965ded97874 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 13 Feb 2025 08:32:12 -0800 Subject: [PATCH 2/3] . --- e2e/tests-dfx/error_context.bash | 5 ++--- src/dfx/src/lib/builders/assets.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/e2e/tests-dfx/error_context.bash b/e2e/tests-dfx/error_context.bash index a09aaf365d..06481684f6 100644 --- a/e2e/tests-dfx/error_context.bash +++ b/e2e/tests-dfx/error_context.bash @@ -138,12 +138,11 @@ teardown() { PATH="$helpers_path" assert_command_fail "$dfx_path" deploy npm_missing # expect to see the npm command line - assert_contains 'program: "npm"' - assert_match 'args: \[.*"npm".*"run".*"build".*\]' + assert_match 'npm run build' # expect to see the name of the canister assert_match "npm_missing" # expect to see the underlying cause - assert_match "No such file or directory" + assert_match "(Is it installed?)" } @test "missing asset source directory" { diff --git a/src/dfx/src/lib/builders/assets.rs b/src/dfx/src/lib/builders/assets.rs index 4c2c59bcd5..bd1bc57412 100644 --- a/src/dfx/src/lib/builders/assets.rs +++ b/src/dfx/src/lib/builders/assets.rs @@ -244,7 +244,7 @@ fn build_frontend( e.into() }; return Err(root_err).context(format!( - "Error executing \"{}\" {}", + "Error executing {} {}", shell_words::quote(&cmd.get_program().to_string_lossy()), cmd.get_args() .map(|a| shell_words::quote(&a.to_string_lossy()).into_owned()) From d6a1087913c2175434cf75ac4e03a899f2bda99c Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Fri, 14 Feb 2025 07:41:48 -0800 Subject: [PATCH 3/3] . --- e2e/tests-dfx/assetscanister.bash | 17 ++++++++--------- src/canisters/frontend/ic-asset/src/sync.rs | 11 +++++++---- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/e2e/tests-dfx/assetscanister.bash b/e2e/tests-dfx/assetscanister.bash index 9aa103a641..0a32e774c9 100644 --- a/e2e/tests-dfx/assetscanister.bash +++ b/e2e/tests-dfx/assetscanister.bash @@ -1382,8 +1382,7 @@ EOF echo '[]' > src/e2e_project_frontend/assets/.ic-assets.json5 assert_command dfx deploy - assert_contains "This project does not define a security policy for some assets." - assert_contains "Assets without any security policy: all" + assert_contains "This project does not define a security policy for any assets." assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_not_match "content-security-policy" assert_not_match "permissions-policy" @@ -1414,7 +1413,7 @@ EOF ]' > src/e2e_project_frontend/assets/.ic-assets.json5 assert_command dfx deploy - assert_not_contains "This project does not define a security policy for some assets." + assert_not_contains "This project does not define a security policy for any assets." assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_not_match "content-security-policy" assert_not_match "permissions-policy" @@ -1428,8 +1427,8 @@ EOF ]' > src/e2e_project_frontend/assets/.ic-assets.json5 assert_command dfx deploy - assert_not_contains "This project does not define a security policy for some assets." - assert_not_contains "This project uses the default security policy for some assets." + assert_not_contains "This project does not define a security policy for any assets." + assert_not_contains "This project uses the default security policy for all assets." assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_not_match "content-security-policy" assert_not_match "permissions-policy" @@ -1443,7 +1442,7 @@ EOF ]' > src/e2e_project_frontend/assets/.ic-assets.json5 assert_command dfx deploy - assert_contains "This project uses the default security policy for some assets." + assert_contains "This project uses the default security policy for all assets." assert_not_contains "Unhardened assets:" assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_match "content-security-policy" @@ -1480,7 +1479,7 @@ EOF ]' > src/e2e_project_frontend/assets/.ic-assets.json5 assert_command dfx deploy - assert_not_contains "This project uses the default security policy for some assets." + assert_not_contains "This project uses the default security policy for all assets." assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_match "content-security-policy" assert_match "permissions-policy" @@ -1508,8 +1507,8 @@ EOF ]' > src/e2e_project_frontend/assets/.ic-assets.json5 assert_command dfx deploy - assert_not_contains "This project does not define a security policy for some assets." - assert_not_contains "This project uses the default security policy for some assets." + assert_not_contains "This project does not define a security policy for any assets." + assert_not_contains "This project uses the default security policy for all assets." assert_command curl --fail --head "http://localhost:$PORT/thing.json?canisterId=$ID" assert_match "content-security-policy: overwritten" assert_match "permissions-policy" diff --git a/src/canisters/frontend/ic-asset/src/sync.rs b/src/canisters/frontend/ic-asset/src/sync.rs index d1453f8a86..36b890de5e 100644 --- a/src/canisters/frontend/ic-asset/src/sync.rs +++ b/src/canisters/frontend/ic-asset/src/sync.rs @@ -384,9 +384,14 @@ pub(crate) fn gather_asset_descriptors( .filter(|asset| asset.config.warn_about_no_security_policy()) .collect_vec(); if !no_policy_assets.is_empty() { + let qnt = if no_policy_assets.len() == asset_descriptors.len() { + "any" + } else { + "some" + }; warn!( logger, - "This project does not define a security policy for some assets." + "This project does not define a security policy for {qnt} assets." ); warn!( logger, @@ -399,9 +404,7 @@ pub(crate) fn gather_asset_descriptors( warn!(logger, " }}"); warn!(logger, "]"); - if no_policy_assets.len() == asset_descriptors.len() { - warn!(logger, "Assets without any security policy: all"); - } else { + if no_policy_assets.len() != asset_descriptors.len() { warn!(logger, "Assets without any security policy:"); for asset in &no_policy_assets { warn!(logger, " - {}", asset.key);