Skip to content
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

Set permissions on the Grafana Agent [Flow] folder... #6540

Merged
merged 9 commits into from
Feb 29, 2024

Conversation

erikbaranowski
Copy link
Contributor

when installing via the windows installer rather than relying on the parent folder permissions.

PR Description

Which issue(s) this PR fixes

Fixes #6522

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

…er rather than relying on the parent folder permissions.

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
CHANGELOG.md Outdated Show resolved Hide resolved
"mkdir" -p dist
curl -O https://nsis.sourceforge.io/mediawiki/images/4/4a/AccessControl.zip && unzip AccessControl.zip -d /usr/share/nsis/ && cp /usr/share/nsis/Plugins/i386-unicode/AccessControl.dll /usr/share/nsis/Plugins/x86-unicode/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can probably be done in the build image instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels semi reasonable :)

AccessControl::ClearOnFile $INSTDIR "Administrators" "FullAccess"
AccessControl::SetOnFile $INSTDIR "SYSTEM" "FullAccess"
AccessControl::GrantOnFile $INSTDIR "Everyone" "ListDirectory"
AccessControl::GrantOnFile $INSTDIR "Everyone" "GenericExecute"
Copy link
Collaborator

Choose a reason for hiding this comment

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

GenericExecute sounds scary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also hate that GrantOnFile is both file and directory, made me look up the docs on this.

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Tested and looks good

…on every build

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Lgtm

@erikbaranowski erikbaranowski merged commit 9e4d3b5 into main Feb 29, 2024
10 checks passed
@erikbaranowski erikbaranowski deleted the windows-installer-security-fix branch February 29, 2024 20:02
rfratto pushed a commit to rfratto/agent that referenced this pull request Mar 5, 2024
* Set permissions on the  folder when installing via the windows installer rather than relying on the parent folder permissions.

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>

---------

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
(cherry picked from commit 9e4d3b5)
rfratto added a commit that referenced this pull request Mar 5, 2024
* Set permissions on the Grafana Agent [Flow] folder... (#6540)

* Set permissions on the  folder when installing via the windows installer rather than relying on the parent folder permissions.

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>

---------

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
(cherry picked from commit 9e4d3b5)

* Loader reuse existing nodes on reload (#6288)

* Move UpdateBlock from componentNode interface to blockNode interface.

This change means that all blockNodes have now the possibility to update their managed block with the update River block content.

* Update the loader to update the managed block of a config node  on reload if it already existed in the graph.
With this optimization, we re-use existing nodes and update them instead of creating a new node.
This is especially useful for modules.

* add two new tests to check that on reload the config nodes behave as expected

* add updateblock to declare node

* update loader logic to detect duplicated blocks and reuse already defined blocks

* add updateblock to import config node

* update changelog

* move function and remove unnecessary check

(cherry picked from commit 2e9d5a2)

* fix(static/metrics/instance): fix duplicate metrics registration panic when recreating the instance (#6608)

Signed-off-by: hainenber <dotronghai96@gmail.com>
(cherry picked from commit 7a61067)

* chore(build): upgrade base image to frequently updated ECR-hosted Ubuntu (#6612)

Signed-off-by: hainenber <dotronghai96@gmail.com>
(cherry picked from commit fe513a4)

* prepare for 0.40.2 release (#6619)

(cherry picked from commit ed54148)

* Port promtail changes part 1 (#6559)

* Port promtail changes part 1

* changelog

(cherry picked from commit 1a642cf)

* remove accidentally committed web/ui/build folder

* fix issue with cherry-picking CHANGELOG.md

* fix bad conflict resolution

* fix test which failed due to bad merge conflict resolution

* On new windows installs, remove default read permissions from agent c… (#6622)

* On new windows installs, remove default read permissions from agent config

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>

* only apply permissions for a new install

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>

* Update CHANGELOG.md

Co-authored-by: Robert Fratto <robertfratto@gmail.com>

---------

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Co-authored-by: Robert Fratto <robertfratto@gmail.com>
(cherry picked from commit e8a3d29)

---------

Co-authored-by: Erik Baranowski <39704712+erikbaranowski@users.noreply.github.com>
Co-authored-by: William Dumont <william.dumont@grafana.com>
Co-authored-by: Đỗ Trọng Hải <41283691+hainenber@users.noreply.github.com>
Co-authored-by: Piotr <17101802+thampiotr@users.noreply.github.com>
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Mar 31, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 31, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure ACLs are set correctly when using non-default install location on windows.
3 participants