Skip to content

Commit

Permalink
chore: Misc output changes (#4109)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamspofford-dfinity committed Feb 18, 2025
1 parent e9b0c14 commit 446010c
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 29 deletions.
19 changes: 9 additions & 10 deletions e2e/tests-dfx/assetscanister.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
5 changes: 2 additions & 3 deletions e2e/tests-dfx/error_context.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
22 changes: 14 additions & 8 deletions src/canisters/frontend/ic-asset/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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);
Expand Down
11 changes: 6 additions & 5 deletions src/dfx/src/commands/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
Expand Down
23 changes: 20 additions & 3 deletions src/dfx/src/lib/builders/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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:?}",),
Expand Down

0 comments on commit 446010c

Please sign in to comment.