From 375463b6d9cf15504d4fb297a06c890449f11c28 Mon Sep 17 00:00:00 2001 From: Adam Spofford <93943719+adamspofford-dfinity@users.noreply.github.com> Date: Fri, 14 Feb 2025 08:32:29 -0800 Subject: [PATCH] chore: Misc output changes (#4109) --- e2e/tests-dfx/assetscanister.bash | 19 ++++++++--------- e2e/tests-dfx/error_context.bash | 5 ++--- src/canisters/frontend/ic-asset/src/sync.rs | 22 +++++++++++++------- src/dfx/src/commands/deploy.rs | 11 +++++----- src/dfx/src/lib/builders/assets.rs | 23 ++++++++++++++++++--- 5 files changed, 51 insertions(+), 29 deletions(-) diff --git a/e2e/tests-dfx/assetscanister.bash b/e2e/tests-dfx/assetscanister.bash index c4dfb79bea..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,8 +1442,8 @@ 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 "Unhardened assets: all" + 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" assert_match "permissions-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/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/canisters/frontend/ic-asset/src/sync.rs b/src/canisters/frontend/ic-asset/src/sync.rs index b17379e4ad..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); @@ -413,11 +416,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..bd1bc57412 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:?}",),