Skip to content
New issue

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

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

Already on GitHub? # to your account

add no-sandbox flag #3863

Closed

Conversation

dogancanbakir
Copy link
Member

@dogancanbakir dogancanbakir commented Jun 22, 2023

Proposed changes

The PR removes the sandbox flag and makes the sandbox mode default. In addition, it adds a no-sandbox flag to allow users to turn off the default sandbox mode if they choose to do so. Closes #3784.

Checklist

  • Pull request is created against the dev branch
  • All checks passed (lint, unit/integration/regression tests etc.) with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@dogancanbakir dogancanbakir self-assigned this Jun 22, 2023
@dogancanbakir dogancanbakir linked an issue Jun 22, 2023 that may be closed by this pull request
@dogancanbakir dogancanbakir requested a review from ehsandeep June 22, 2023 13:06
@ehsandeep ehsandeep added the Status: In Progress This issue is being worked on, and has someone assigned. label Jun 23, 2023
@ehsandeep ehsandeep deleted the branch projectdiscovery:dev June 27, 2023 16:30
@ehsandeep ehsandeep closed this Jun 27, 2023
@ehsandeep ehsandeep reopened this Jun 27, 2023
Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

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

failing test

@ehsandeep ehsandeep added the Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity. label Jul 3, 2023
@dogancanbakir dogancanbakir requested a review from ehsandeep July 3, 2023 10:02
@ehsandeep ehsandeep requested a review from Mzack9999 July 3, 2023 11:47
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

Implementation: lgtm
Note: this change will prevent nuclei from connecting to private ipv4/ipv6 ranges, making it unusable within VPC, CI/CD, and internal pipelines. I recommend evaluating carefully before proceeding (a possible solution could be to implement different level of sandboxing with still internal ranges enabled by default).

@ehsandeep ehsandeep added Status: On Hold Similar to blocked, but is assigned to someone Status: Review Needed The issue has a PR attached to it which needs to be reviewed and removed Status: In Progress This issue is being worked on, and has someone assigned. labels Jul 3, 2023
Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

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

since sandbox component involves, disabling both can cause issues for default behavior, we can expose file and network options with separate CLI options to have better control and nonbreaking changes.

   -lfa, -allow-local-file-access            allows file (payload) access anywhere on the system
   -lna, -restrict-local-network-access      blocks connections to the local / private network

@ehsandeep ehsandeep added Status: Revision Needed Submitter of PR needs to revise the PR related to the issue. and removed Status: On Hold Similar to blocked, but is assigned to someone Status: Review Needed The issue has a PR attached to it which needs to be reviewed labels Jul 8, 2023
@ehsandeep
Copy link
Member

Suppressed by #3927

@ehsandeep ehsandeep closed this Jul 14, 2023
@ehsandeep ehsandeep removed the Status: Revision Needed Submitter of PR needs to revise the PR related to the issue. label Jul 14, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Type: Maintenance Updating phrasing or wording to make things clearer or removing ambiguity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enabling sandbox option as default
3 participants