Skip to content

fix: reject path-like network and environment names - #795

Merged
raymondk merged 1 commit into
mainfrom
fix/validate-network-environment-names
Sep 28, 2026
Merged

raymondk merged 1 commit into
mainfrom
fix/validate-network-environment-names

Conversation

@raymondk

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI balanced review requested due to automatic review settings September 25, 2026 23:27
@raymondk
raymondk requested a review from a team as a code owner September 25, 2026 23:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The changelog does not clearly disclose the breaking manifest restriction, and the validation documentation is stale.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
What changed in this PR

Rejects unsafe network and environment names before they become filesystem path components.

Changes:

  • Adds validation and dedicated errors for invalid names.
  • Adds regression coverage for path-like names.
  • Adds an Unreleased changelog entry.
File Description
crates/​icp-project/​src/​project.rs Validates names and tests rejected inputs.
CHANGELOG.md Records the behavior change.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread CHANGELOG.md

# Unreleased

fix: reject path-like network and environment names

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think it's fine to leave this as non breaking.

Comment thread crates/icp-project/src/project.rs Outdated
Comment on lines +336 to +338
/// archive paths — so no per-site sanitizing is needed. Network and environment
/// names are held to it too, since each becomes a path component under `.icp`
/// (and `/`, `..` or an absolute name would escape it). In particular `:` is the
Network and environment names are used as path components under `.icp`
(the network's state directory and `<env>.ids.json`), but were never
validated.

Hold both to the same rule canister names already follow: non-empty,
ASCII letters, digits, `_` and `-` only.
@raymondk
raymondk force-pushed the fix/validate-network-environment-names branch from 442cb68 to ebd1b5c Compare September 25, 2026 23:40
@raymondk
raymondk added this pull request to stack #797 September 25, 2026 23:42
@raymondk
raymondk merged commit 111ab10 into main Sep 28, 2026
109 checks passed
@raymondk
raymondk deleted the fix/validate-network-environment-names branch September 28, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants