Skip to content

AdGuard Home : security and logging improvment #5817

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

Closed
wants to merge 1 commit into from

Conversation

cweiland
Copy link

@cweiland cweiland commented Jul 7, 2025

✍️ Description

  • Create a dedicated user (adguardhome)
  • Set needed rights to listen on dns port
  • Add user on service file
  • Logs are now on journald

🔗 Related PR / Issue

Link: #

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected.
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing functionality in a way that may require updates.
  • 🆕 New script – A fully functional and tested script or script set.
  • 🌍 Website update – Changes to website-related JSON files or metadata.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, AppName.md, CONTRIBUTING.md, or other docs.

- Create a dedicated user (adguardhome)
- Set needed rights to listen on dns port
- Add user on service file
- Logs are now on journald
@cweiland cweiland requested a review from a team as a code owner July 7, 2025 20:45
@github-actions github-actions bot added the update script A change that updates a script label Jul 7, 2025
Copy link
Member

@michelroegl-brunner michelroegl-brunner left a comment

Choose a reason for hiding this comment

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

Why is this needed? Dose it no longer work with root? We keep all installs as root user for a consitency reason.

Comment on lines -30 to -33
ExecStart=/opt/AdGuardHome/AdGuardHome "-s" "run"
ExecStart=/opt/AdGuardHome/AdGuardHome "-s" "run" "-l" "syslog"
WorkingDirectory=/opt/AdGuardHome
StandardOutput=file:/var/log/AdGuardHome.out
StandardError=file:/var/log/AdGuardHome.err
Copy link
Member

Choose a reason for hiding this comment

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

is there any added benefit of this?

Copy link
Author

Choose a reason for hiding this comment

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

is easier to manage logs with journald than plain files
ex : rotating logs, search, ...

Copy link
Member

Choose a reason for hiding this comment

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

agreed on this one

@cweiland
Copy link
Author

cweiland commented Jul 8, 2025

Why is this needed? Dose it no longer work with root? We keep all installs as root user for a consitency reason.

it's better to run it with a dedicated account to avoid privileges escalation issues

@michelroegl-brunner
Copy link
Member

We have discussed this internaly with all maintainers and our conclusion was that we dont want to start to create individual users for all apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update script A change that updates a script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants