-
Notifications
You must be signed in to change notification settings - Fork 42
Add default HTTP domain permissions following policy-mcp design #368
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements default HTTP domain permissions following the policy-mcp design, allowing components to use defaults: true
in network permissions to automatically access 75+ commonly used domains instead of manually listing each one.
Key changes:
- Added
NetworkDefaultsPermission
struct andDefaults
variant to handle default domain inclusions - Created
defaults.rs
module with curated list of 75+ commonly used domains across package registries, version control, cloud providers, AI APIs, and CDNs - Enhanced validation to accept defaults in allow lists while rejecting them in deny lists
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
crates/policy/src/defaults.rs | New module defining 75+ default HTTP domains for common development services |
crates/policy/src/types.rs | Added NetworkDefaultsPermission struct and Defaults enum variant with validation logic |
crates/policy/src/lib.rs | Exported get_default_domains function from new defaults module |
crates/wassette/src/wasistate.rs | Enhanced extract_allowed_hosts to expand defaults and added comprehensive tests |
crates/wassette/src/lib.rs | Added integration test for WasiStateTemplate with defaults functionality |
docs/using/permissions.md | Updated documentation with examples and explanations of default domains feature |
crates/policy/testdata/network-defaults.yaml | Example policy file demonstrating defaults usage |
CHANGELOG.md | Added entry documenting the new default HTTP domain permissions feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
crates/policy/src/defaults.rs
Outdated
pub fn get_default_domains() -> Vec<&'static str> { | ||
vec![ | ||
// Package Registries | ||
"registry.npmjs.org", | ||
"*.npmjs.com", | ||
"pypi.org", | ||
"*.pypi.org", | ||
"files.pythonhosted.org", | ||
"rubygems.org", | ||
"*.rubygems.org", | ||
"crates.io", | ||
"*.crates.io", | ||
"static.crates.io", | ||
"index.crates.io", | ||
"nuget.org", | ||
"*.nuget.org", | ||
"api.nuget.org", | ||
"repo.maven.apache.org", | ||
"repo1.maven.org", | ||
"central.maven.org", | ||
"search.maven.org", | ||
"registry.yarnpkg.com", | ||
// Version Control Systems | ||
"github.com", | ||
"*.github.com", | ||
"api.github.com", | ||
"raw.githubusercontent.com", | ||
"codeload.github.com", | ||
"gitlab.com", | ||
"*.gitlab.com", | ||
"bitbucket.org", | ||
"*.bitbucket.org", | ||
"api.bitbucket.org", | ||
// Cloud Service Providers | ||
"*.amazonaws.com", | ||
"s3.amazonaws.com", | ||
"*.s3.amazonaws.com", | ||
"*.googleapis.com", | ||
"storage.googleapis.com", | ||
"*.google.com", | ||
"*.azure.com", | ||
"*.azurewebsites.net", | ||
"*.blob.core.windows.net", | ||
"*.cloudflare.com", | ||
"cloudflare.com", | ||
// Container Registries | ||
"docker.io", | ||
"*.docker.io", | ||
"registry-1.docker.io", | ||
"index.docker.io", | ||
"quay.io", | ||
"*.quay.io", | ||
"ghcr.io", | ||
"*.pkg.dev", | ||
"gcr.io", | ||
"*.gcr.io", | ||
// AI and ML APIs | ||
"api.openai.com", | ||
"*.openai.com", | ||
"api.anthropic.com", | ||
"*.anthropic.com", | ||
"api.cohere.ai", | ||
"*.cohere.ai", | ||
"huggingface.co", | ||
"*.huggingface.co", | ||
"cdn-lfs.huggingface.co", | ||
// Content Delivery Networks (CDNs) | ||
"cdn.jsdelivr.net", | ||
"*.jsdelivr.net", | ||
"unpkg.com", | ||
"cdnjs.cloudflare.com", | ||
"*.fastly.net", | ||
"*.akamaized.net", | ||
"*.edgecastcdn.net", | ||
// Documentation and Learning Resources | ||
"docs.rs", | ||
"readthedocs.io", | ||
"*.readthedocs.io", | ||
"readthedocs.org", | ||
"*.readthedocs.org", | ||
// Build and CI/CD Services | ||
"circleci.com", | ||
"*.circleci.com", | ||
"actions.githubusercontent.com", | ||
"objects.githubusercontent.com", | ||
] |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider returning &[&'static str]
instead of Vec<&'static str>
to avoid heap allocation on every call. Since the domains are static, a slice reference would be more efficient.
pub fn get_default_domains() -> Vec<&'static str> { | |
vec![ | |
// Package Registries | |
"registry.npmjs.org", | |
"*.npmjs.com", | |
"pypi.org", | |
"*.pypi.org", | |
"files.pythonhosted.org", | |
"rubygems.org", | |
"*.rubygems.org", | |
"crates.io", | |
"*.crates.io", | |
"static.crates.io", | |
"index.crates.io", | |
"nuget.org", | |
"*.nuget.org", | |
"api.nuget.org", | |
"repo.maven.apache.org", | |
"repo1.maven.org", | |
"central.maven.org", | |
"search.maven.org", | |
"registry.yarnpkg.com", | |
// Version Control Systems | |
"github.com", | |
"*.github.com", | |
"api.github.com", | |
"raw.githubusercontent.com", | |
"codeload.github.com", | |
"gitlab.com", | |
"*.gitlab.com", | |
"bitbucket.org", | |
"*.bitbucket.org", | |
"api.bitbucket.org", | |
// Cloud Service Providers | |
"*.amazonaws.com", | |
"s3.amazonaws.com", | |
"*.s3.amazonaws.com", | |
"*.googleapis.com", | |
"storage.googleapis.com", | |
"*.google.com", | |
"*.azure.com", | |
"*.azurewebsites.net", | |
"*.blob.core.windows.net", | |
"*.cloudflare.com", | |
"cloudflare.com", | |
// Container Registries | |
"docker.io", | |
"*.docker.io", | |
"registry-1.docker.io", | |
"index.docker.io", | |
"quay.io", | |
"*.quay.io", | |
"ghcr.io", | |
"*.pkg.dev", | |
"gcr.io", | |
"*.gcr.io", | |
// AI and ML APIs | |
"api.openai.com", | |
"*.openai.com", | |
"api.anthropic.com", | |
"*.anthropic.com", | |
"api.cohere.ai", | |
"*.cohere.ai", | |
"huggingface.co", | |
"*.huggingface.co", | |
"cdn-lfs.huggingface.co", | |
// Content Delivery Networks (CDNs) | |
"cdn.jsdelivr.net", | |
"*.jsdelivr.net", | |
"unpkg.com", | |
"cdnjs.cloudflare.com", | |
"*.fastly.net", | |
"*.akamaized.net", | |
"*.edgecastcdn.net", | |
// Documentation and Learning Resources | |
"docs.rs", | |
"readthedocs.io", | |
"*.readthedocs.io", | |
"readthedocs.org", | |
"*.readthedocs.org", | |
// Build and CI/CD Services | |
"circleci.com", | |
"*.circleci.com", | |
"actions.githubusercontent.com", | |
"objects.githubusercontent.com", | |
] | |
/// The static list of default HTTP domains | |
static DEFAULT_DOMAINS: &[&str] = &[ | |
// Package Registries | |
"registry.npmjs.org", | |
"*.npmjs.com", | |
"pypi.org", | |
"*.pypi.org", | |
"files.pythonhosted.org", | |
"rubygems.org", | |
"*.rubygems.org", | |
"crates.io", | |
"*.crates.io", | |
"static.crates.io", | |
"index.crates.io", | |
"nuget.org", | |
"*.nuget.org", | |
"api.nuget.org", | |
"repo.maven.apache.org", | |
"repo1.maven.org", | |
"central.maven.org", | |
"search.maven.org", | |
"registry.yarnpkg.com", | |
// Version Control Systems | |
"github.com", | |
"*.github.com", | |
"api.github.com", | |
"raw.githubusercontent.com", | |
"codeload.github.com", | |
"gitlab.com", | |
"*.gitlab.com", | |
"bitbucket.org", | |
"*.bitbucket.org", | |
"api.bitbucket.org", | |
// Cloud Service Providers | |
"*.amazonaws.com", | |
"s3.amazonaws.com", | |
"*.s3.amazonaws.com", | |
"*.googleapis.com", | |
"storage.googleapis.com", | |
"*.google.com", | |
"*.azure.com", | |
"*.azurewebsites.net", | |
"*.blob.core.windows.net", | |
"*.cloudflare.com", | |
"cloudflare.com", | |
// Container Registries | |
"docker.io", | |
"*.docker.io", | |
"registry-1.docker.io", | |
"index.docker.io", | |
"quay.io", | |
"*.quay.io", | |
"ghcr.io", | |
"*.pkg.dev", | |
"gcr.io", | |
"*.gcr.io", | |
// AI and ML APIs | |
"api.openai.com", | |
"*.openai.com", | |
"api.anthropic.com", | |
"*.anthropic.com", | |
"api.cohere.ai", | |
"*.cohere.ai", | |
"huggingface.co", | |
"*.huggingface.co", | |
"cdn-lfs.huggingface.co", | |
// Content Delivery Networks (CDNs) | |
"cdn.jsdelivr.net", | |
"*.jsdelivr.net", | |
"unpkg.com", | |
"cdnjs.cloudflare.com", | |
"*.fastly.net", | |
"*.akamaized.net", | |
"*.edgecastcdn.net", | |
// Documentation and Learning Resources | |
"docs.rs", | |
"readthedocs.io", | |
"*.readthedocs.io", | |
"readthedocs.org", | |
"*.readthedocs.org", | |
// Build and CI/CD Services | |
"circleci.com", | |
"*.circleci.com", | |
"actions.githubusercontent.com", | |
"objects.githubusercontent.com", | |
]; | |
pub fn get_default_domains() -> &'static [&'static str] { | |
DEFAULT_DOMAINS |
Copilot uses AI. Check for mistakes.
crates/wassette/src/wasistate.rs
Outdated
if let Ok(json_value) = serde_json::to_value(allow_entry) { | ||
// Check for defaults field | ||
if let Some(defaults_value) = json_value.get("defaults") { | ||
if defaults_value.as_bool().unwrap_or(false) { |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting the boolean check into a variable for better readability: let is_defaults_enabled = defaults_value.as_bool().unwrap_or(false);
if defaults_value.as_bool().unwrap_or(false) { | |
let is_defaults_enabled = defaults_value.as_bool().unwrap_or(false); | |
if is_defaults_enabled { |
Copilot uses AI. Check for mistakes.
crates/policy/src/types.rs
Outdated
match perm { | ||
NetworkPermission::Defaults(_) => { | ||
// Defaults should not be used in deny lists | ||
bail!("'defaults' cannot be used in deny lists"); |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message could be more descriptive. Consider: 'defaults: true' is not allowed in network deny lists - defaults can only be used in allow lists
bail!("'defaults' cannot be used in deny lists"); | |
bail!("'defaults: true' is not allowed in network deny lists - defaults can only be used in allow lists"); |
Copilot uses AI. Check for mistakes.
@copilot run |
@copilot resolve all the review comments |
Done. Formatted the code with |
Resolved all review comments in commit 8f3a2b1:
|
7cf5f22
to
466ad9f
Compare
crates/policy/src/defaults.rs
Outdated
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[test] | ||
fn test_default_domains_not_empty() { | ||
let domains = get_default_domains(); | ||
assert!( | ||
!domains.is_empty(), | ||
"Default domains list should not be empty" | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_default_domains_include_common_registries() { | ||
let domains = get_default_domains(); | ||
|
||
// Check for major package registries | ||
assert!( | ||
domains.contains(&"registry.npmjs.org"), | ||
"Should include npm registry" | ||
); | ||
assert!( | ||
domains.contains(&"pypi.org"), | ||
"Should include Python package index" | ||
); | ||
assert!( | ||
domains.contains(&"crates.io"), | ||
"Should include Rust crates registry" | ||
); | ||
assert!( | ||
domains.contains(&"nuget.org"), | ||
"Should include NuGet registry" | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_default_domains_include_vcs() { | ||
let domains = get_default_domains(); | ||
|
||
// Check for version control systems | ||
assert!(domains.contains(&"github.com"), "Should include GitHub"); | ||
assert!(domains.contains(&"gitlab.com"), "Should include GitLab"); | ||
assert!( | ||
domains.contains(&"bitbucket.org"), | ||
"Should include Bitbucket" | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_default_domains_include_cloud_providers() { | ||
let domains = get_default_domains(); | ||
|
||
// Check for major cloud providers | ||
assert!(domains.contains(&"*.amazonaws.com"), "Should include AWS"); | ||
assert!( | ||
domains.contains(&"*.googleapis.com"), | ||
"Should include Google Cloud" | ||
); | ||
assert!(domains.contains(&"*.azure.com"), "Should include Azure"); | ||
} | ||
|
||
#[test] | ||
fn test_default_domains_include_ai_apis() { | ||
let domains = get_default_domains(); | ||
|
||
// Check for AI/ML APIs | ||
assert!(domains.contains(&"api.openai.com"), "Should include OpenAI"); | ||
assert!( | ||
domains.contains(&"api.anthropic.com"), | ||
"Should include Anthropic" | ||
); | ||
assert!( | ||
domains.contains(&"huggingface.co"), | ||
"Should include Hugging Face" | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_default_domains_no_duplicates() { | ||
let domains = get_default_domains(); | ||
let mut unique_domains = std::collections::HashSet::new(); | ||
|
||
for domain in domains { | ||
assert!( | ||
unique_domains.insert(domain), | ||
"Duplicate domain found: {}", | ||
domain | ||
); | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_default_domains_all_lowercase() { | ||
let domains = get_default_domains(); | ||
|
||
for domain in domains { | ||
assert_eq!( | ||
*domain, | ||
domain.to_lowercase(), | ||
"Domain {} should be lowercase", | ||
domain | ||
); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please remove those tests @copilot ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the tests from defaults.rs in commit 0a8d7e5. All remaining tests still pass (42 tests in policy crate, 34 tests in wassette wasistate module).
@copilot can you rebase and resolve the conflict in docs/using/permissions.md |
- Add NetworkDefaultsPermission type to policy NetworkPermission enum - Implement get_default_domains() with 75+ commonly used domains - Update extract_allowed_hosts() to expand defaults when specified - Add comprehensive tests for defaults functionality - Update documentation to explain defaults feature - Add example policy file demonstrating defaults usage Co-authored-by: Mossaka <[email protected]>
Co-authored-by: Mossaka <[email protected]>
Co-authored-by: Mossaka <[email protected]>
…rror messages - Change get_default_domains() to return &'static [&'static str] instead of Vec to avoid heap allocation - Extract boolean check into variable for better readability - Improve error message for defaults in deny lists to be more descriptive Co-authored-by: Mossaka <[email protected]>
Co-authored-by: Mossaka <[email protected]>
79e199c
to
08ae253
Compare
@copilot could you please run |
Co-authored-by: Mossaka <[email protected]>
Formatted the code with |
Signed-off-by: Jiaxiao Zhou <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should hold off on merging this until after we've had an opportunity to sync on this offline.
Implementation: Add default HTTP domain permissions
Following the design in https://github.com/microsoft/policy-mcp/blob/main/DEFAULTS.md, this PR adds default domain support so components don't need to request network permission grants for commonly used domains.
Summary of Changes
Policy Crate (
crates/policy/
):NetworkDefaultsPermission
struct withdefaults: bool
fieldNetworkPermission
enum to include newDefaults
variant (placed first for proper serde untagged parsing)defaults.rs
module withget_default_domains()
function returning&'static [&'static str]
for efficiency (no heap allocation)testdata/network-defaults.yaml
Wassette Crate (
crates/wassette/
):extract_allowed_hosts()
to:defaults: true
in network permissionsDocumentation (
docs/
):docs/reference/permissions.md
with:Changelog:
CHANGELOG.md
with feature descriptionDefault Domains Included (75+ domains)
The implementation includes all domains from the policy-mcp design:
Test Results
All tests pass successfully:
cargo +nightly fmt
Usage Example
Files Changed
CHANGELOG.md
- Added feature descriptioncrates/policy/src/defaults.rs
- New module with default domains list (optimized as static slice)crates/policy/src/lib.rs
- Export defaults modulecrates/policy/src/types.rs
- Added NetworkDefaultsPermission type and validation with improved error messagescrates/policy/testdata/network-defaults.yaml
- Example policy filecrates/wassette/src/wasistate.rs
- Updated to expand defaults with improved readability + testscrates/wassette/src/lib.rs
- Added integration test + formatting fixesdocs/reference/permissions.md
- Updated documentationSecurity Considerations
allow
lists (rejected indeny
lists)Rebase Notes
Successfully rebased onto main branch. The permissions documentation file was moved from
docs/using/permissions.md
todocs/reference/permissions.md
as part of PR #370 in main, and this change has been integrated into the PR.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.