Skip to content

Fix/613 default network policy#630

Merged
utksh1 merged 1 commit into
utksh1:mainfrom
ionfwsrijan:fix/613-default-network-policy
Jun 7, 2026
Merged

Fix/613 default network policy#630
utksh1 merged 1 commit into
utksh1:mainfrom
ionfwsrijan:fix/613-default-network-policy

Conversation

@ionfwsrijan
Copy link
Copy Markdown
Contributor

Description

settings.network_allowlist defaults to [] (empty list). When the allowlist is empty, _init_default_policies() automatically adds 0.0.0.0/0 and ::/0 as "Default allow all" rules. This means every network connection from any scanner to any public internet host is permitted unless the operator explicitly sets SECUSCAN_NETWORK_ALLOWLIST. The deny-by-default security model is illusory.

Fix: remove the default allow-all rules. An empty allowlist now means deny-all egress. Operators must explicitly configure SECUSCAN_NETWORK_ALLOWLIST to permit outbound traffic.

Related Issues

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

Verified that _init_default_policies no longer adds 0.0.0.0/0 or ::/0 when network_allowlist is empty. The denylist (RFC 1918, cloud metadata) continues to be applied as before.

Checklist

  • My code follows the code style of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.

@ionfwsrijan ionfwsrijan force-pushed the fix/613-default-network-policy branch 2 times, most recently from 0314185 to 8f73371 Compare June 6, 2026 08:31
@ionfwsrijan
Copy link
Copy Markdown
Contributor Author

@utksh1 You may review and merge

@utksh1 utksh1 added level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:security Security work category bonus label area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests labels Jun 7, 2026
Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

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

Focused default network policy fix with green checks. Approving for merge.

@utksh1 utksh1 added gssoc:approved Admin validation: approved for GSSoC scoring and removed gssoc:approved Admin validation: approved for GSSoC scoring labels Jun 7, 2026
Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

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

After #629 merged, this branch now conflicts with main because it carries the same CI baseline files. The default-network-policy fix is useful, but this PR is not mergeable now. Please rebase on latest main and keep the branch scoped to config.py/network_policy.py plus directly related tests.

@ionfwsrijan
Copy link
Copy Markdown
Contributor Author

@utksh1 You may merge this now

Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

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

Re-reviewed the latest update. The core default-deny network policy change is useful, but the PR still includes unrelated CI-baseline/frontend test changes. Please rebase on latest main and keep this scoped to backend/secuscan/config.py and backend/secuscan/network_policy.py plus direct network-policy tests only.

Previously an empty SECUSCAN_NETWORK_ALLOWLIST caused _init_default_policies() to add 0.0.0.0/0 and ::/0 as allow-all rules. Empty allowlist now means deny-all egress.

Fixes utksh1#613
@ionfwsrijan ionfwsrijan force-pushed the fix/613-default-network-policy branch from 919e3dd to 0e049f8 Compare June 7, 2026 10:12
@ionfwsrijan
Copy link
Copy Markdown
Contributor Author

@utksh1 I've dropped unrelated CI-baseline and frontend test changes. You may review and merge now

Copy link
Copy Markdown
Owner

@utksh1 utksh1 left a comment

Choose a reason for hiding this comment

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

Re-reviewed the latest update. The PR is now focused on default-deny network policy behavior with direct unit coverage and green visible checks. Approving.

@utksh1 utksh1 added the gssoc:approved Admin validation: approved for GSSoC scoring label Jun 7, 2026
@utksh1 utksh1 merged commit 2837d40 into utksh1:main Jun 7, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Backend API, database, or service work area:security Security-sensitive implementation or tests gssoc:approved Admin validation: approved for GSSoC scoring level:advanced 55 pts difficulty label for advanced contributor PRs type:bug Bug fix work category bonus label type:security Security work category bonus label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants