Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Misc output changes #4109

Merged
merged 3 commits into from
Feb 14, 2025
+51 −29
Merged

chore: Misc output changes #4109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0bd2f0d
Misc output changes
adamspofford-dfinity Feb 13, 2025
32905a5
.
adamspofford-dfinity Feb 13, 2025
d6a1087
.
adamspofford-dfinity Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions e2e/tests-dfx/assetscanister.bash
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"recommended" doesn't work on Safari, yes?

Copy link
Contributor Author

@adamspofford-dfinity adamspofford-dfinity Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, hence 'legacy'. I had trimmed that down from 'legacy/safari' because 'safari' implies that the website will actually work with safari, and it won't. Not sure if there's a good way to phrase it without taking up too much space.

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
View file
Open in desktop
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
Loading