-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Wireguard more secure post_up
config described in documentation
#320
Wireguard more secure post_up
config described in documentation
#320
Conversation
WalkthroughThe recent updates to the WireGuard documentation enhance clarity on configuring access control for HomeAssistant devices within a local network. New examples illustrate how to effectively use the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
e08c099
to
ce7013b
Compare
post_up
config described in documentationpost_up
config described in documentation
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
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.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (2)
wireguard/DOCS.md (2)
202-202
: Add a comma after "usually" for clarity.Adding a comma after "usually" will improve the readability of the sentence.
- Suffix `/24` is a way of subnet mask specifying in CIDR, and usually you should not be worried by this. + Suffix `/24` is a way of subnet mask specifying in CIDR, and usually, you should not be worried by this.Tools
LanguageTool
[typographical] ~202-~202: Consider adding a comma after ‘usually’ for more clarity.
Context: ...of subnet mask specifying in CIDR, and usually you should not be worried by this. **...(RB_LY_COMMA)
209-209
: Add a comma after "Finally" for clarity.Adding a comma after "Finally" will improve the readability of the sentence.
- Finally your yaml `post_up` section may look like: + Finally, your yaml `post_up` section may look like:Tools
LanguageTool
[typographical] ~209-~209: Consider adding a comma after ‘Finally’ for more clarity.
Context: ...tstate ESTABLISHED,RELATED -j ACCEPT;Finally your yaml
post_up` section may look li...(RB_LY_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- wireguard/DOCS.md (1 hunks)
Additional context used
LanguageTool
wireguard/DOCS.md
[style] ~179-~179: This phrase is redundant (‘N’ stands for ‘network’). Use simply “LAN”.
Context: ... machine, not all the devices from your LAN network. To do so, you can use this example of ...(ACRONYM_TAUTOLOGY)
[typographical] ~202-~202: Consider adding a comma after ‘usually’ for more clarity.
Context: ...of subnet mask specifying in CIDR, and usually you should not be worried by this. **...(RB_LY_COMMA)
[grammar] ~206-~206: A verb or adverb may be misspelled or missing.
Context: ...ore than your HomeAssistant device, you can just additionaliptables
commands before this command...(PRP_MD_NN)
[typographical] ~209-~209: Consider adding a comma after ‘Finally’ for more clarity.
Context: ...tstate ESTABLISHED,RELATED -j ACCEPT;Finally your yaml
post_up` section may look li...(RB_LY_COMMA)
Markdownlint
wireguard/DOCS.md
204-204: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
IP range would be `192.168.0.0`. Suffix `/24` is a way of subnet mask | ||
specifying in CIDR, and usually you should not be worried by this. | ||
|
||
**Hint** |
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.
Convert emphasis to a heading.
Emphasis should not be used as a heading. Convert it to a proper heading.
- **Hint**
+ ### Hint
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
**Hint** | |
### Hint |
Tools
Markdownlint
204-204: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
|
||
**Hint** | ||
|
||
If you would like to access more than your HomeAssistant device, you can just |
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.
Fix grammatical error.
The phrase "you can just additional iptables
commands" is missing a verb. It should be "you can just add additional iptables
commands".
- If you would like to access more than your HomeAssistant device, you can just additional `iptables` commands before this command:
+ If you would like to access more than your HomeAssistant device, you can just add additional `iptables` commands before this command:
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
If you would like to access more than your HomeAssistant device, you can just | |
If you would like to access more than your HomeAssistant device, you can just add additional `iptables` commands before this command: |
Tools
LanguageTool
[grammar] ~206-~206: A verb or adverb may be misspelled or missing.
Context: ...ore than your HomeAssistant device, you can just additionaliptables
commands before this command...(PRP_MD_NN)
Moreover you would like to access only your HomeAssistant machine, not all | ||
the devices from your LAN network. To do so, you can use this example of |
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.
Correct redundancy in the phrase "LAN network".
The phrase "LAN network" is redundant since "LAN" stands for "Local Area Network". Use "LAN" instead.
- Moreover you would like to access only your HomeAssistant machine, not all the devices from your LAN network.
+ Moreover you would like to access only your HomeAssistant machine, not all the devices from your LAN.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Moreover you would like to access only your HomeAssistant machine, not all | |
the devices from your LAN network. To do so, you can use this example of | |
Moreover you would like to access only your HomeAssistant machine, not all the devices from your LAN. To do so, you can use this example of |
Tools
LanguageTool
[style] ~179-~179: This phrase is redundant (‘N’ stands for ‘network’). Use simply “LAN”.
Context: ... machine, not all the devices from your LAN network. To do so, you can use this example of ...(ACRONYM_TAUTOLOGY)
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
Proposed Changes
I had an issue about accessing every device in my LAN network, and as I wanted to shrink the possible attack surface, and cut off the possibility of accessing for example the router. The Wireguard is really secure, although there is a possibility of your WG client config file to be stolen, and thus I feel safer with single WG client config with control what devices I can access from external network.
I have spent a lot of time to find desired solution, and I would like to share it with the others, so you could be safer with your HomeAssistant :)
Summary by CodeRabbit
post_up
section to restrict access to specific devices.iptables
commands and clarified the use of CIDR notation for subnetting.