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

fix: use temp dir for secret data #1290

Merged
merged 4 commits into from
Jul 22, 2024
Merged

Conversation

dimaqq
Copy link
Contributor

@dimaqq dimaqq commented Jul 19, 2024

Change the mechanism used to pass secret values from ops to hook tools from command line to files in temporary directory.

Removes the 2MB total secret size limit imposed by command line length

Hook tool calls juju agent which in turn reads these files, I've confirmed with the Juju team that juju agent and charm code are run as the same user.

@dimaqq dimaqq changed the title feat: use temp for secret data fix: use temp dir for secret data Jul 19, 2024
@dimaqq
Copy link
Contributor Author

dimaqq commented Jul 19, 2024

Also tested manually by replacing ops in a running charm venv with the content of this branch.

Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Yes, I like this approach a lot better, thanks! The code reads a lot more simply, and it's nice to avoid keeping N files open. If you've done manual testing, I'm good with this.

ops/model.py Show resolved Hide resolved
Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer left a comment

Choose a reason for hiding this comment

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

Did a quick check (Juju 3.6, LXD) and all seems good here too. Agreed that this approach seems generally nicer.

@dimaqq dimaqq merged commit fea6d20 into canonical:main Jul 22, 2024
28 checks passed
@dimaqq dimaqq deleted the temporary-directory branch July 22, 2024 00:46
dimaqq added a commit to dimaqq/operator that referenced this pull request Jul 22, 2024
Change the mechanism used to pass secret values from ops to hook tools
from command line to files in temporary directory.

Removes the 2MB total secret size limit imposed by command line length

Hook tool calls juju agent which in turn reads these files, I've
confirmed with the Juju team that juju agent and charm code are run as
the same user.
@dimaqq
Copy link
Contributor Author

dimaqq commented Jul 23, 2024

Fixes CVE-2024-41129

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants