[WIP] proton_pass lookup: mark plugin return values as secrets by felixfontein · Pull Request #12740 · ansible-collections/community.general · GitHub
Skip to content

[WIP] proton_pass lookup: mark plugin return values as secrets - #12740

Open
felixfontein wants to merge 3 commits into
ansible-collections:mainfrom
felixfontein:secrets-lookup-4
Open

[WIP] proton_pass lookup: mark plugin return values as secrets#12740
felixfontein wants to merge 3 commits into
ansible-collections:mainfrom
felixfontein:secrets-lookup-4

Conversation

@felixfontein

@felixfontein felixfontein commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator
SUMMARY

Mark sensitive return values of lookups and modules as secrets on ansible-core 2.22+. Uses the helper introduced in #12709.

The Secrets API is being introduced for ansible-core 2.22 and allows modules and plugins to mark secret values for redaction. As opposed to the rudimentary no_log handling up until ansible-core 2.21, this improves masking of secret values.

More infos:

For this plugin I have no idea how the return values look like, so this definitely needs help from active users and/or plugin maintainers!

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

plugins/lookup/proton_pass.py

@felixfontein felixfontein added backport-12 Automatically create a backport for the stable-12 branch backport-13 Automatically create a backport for the stable-13 branch labels Sep 15, 2026
@felixfontein felixfontein mentioned this pull request Sep 15, 2026
Open
@ansibullbot

ansibullbot commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug lookup lookup plugin plugins plugin (any type) small_patch Hopefully easy to review labels Sep 15, 2026
@felixfontein

felixfontein commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

@jeankhawand @kingtong I would very much appreciate if you could take a look at this PR, to make sure that only definite secrets are marked as secrets, and not random other values. Thanks.

@kingtong

kingtong commented Sep 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@jeankhawand @kingtong I would very much appreciate if you could take a look at this PR, to make sure that only definite secrets are marked as secrets, and not random other values. Thanks.

I took a quick look and, from what I understand, every string in an item's parsed fields will be registered as a secret, including non-secret fields.

Whether a field is secret could be determined during parsing, while the necessary context is still available. For extra fields, we can distinguish Hidden/Totp from Text/Timestamp; for standard fields, we know the item type and field name, for example Login.password or CreditCard.verification_number.

The values could be registered directly during parsing, but that would have a broader effect because not all parsed fields are necessarily returned. Another option would be for the parser to return the fields as it does currently, together with the set of fields identified as secret. The caller could then register only the secret values it actually returns.

@ansibullbot ansibullbot removed the small_patch Hopefully easy to review label Sep 23, 2026
@ansibullbot ansibullbot added tests tests unit tests/unit labels Sep 23, 2026
@felixfontein felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 23, 2026
@felixfontein

felixfontein commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator Author

@kingtong thanks for your feedback! I've adjusted the module that it internally passes on a boolean flag (True for definite secrets). Can you take a look whether the selection of secrets makes sense? Obviously it won't be perfect for every use-case, but that's impossible anyway I guess :)

@kingtong

kingtong commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

@kingtong thanks for your feedback! I've adjusted the module that it internally passes on a boolean flag (True for definite secrets). Can you take a look whether the selection of secrets makes sense? Obviously it won't be perfect for every use-case, but that's impossible anyway I guess :)

LGTM. I’d suggest maybe adding a test in TestLookupModule to assert that only returned secret values are registered (not field names or non-secret text values).

@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Sep 24, 2026
@ansibullbot ansibullbot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Sep 26, 2026
@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Sep 26, 2026
@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Sep 27, 2026
@felixfontein felixfontein removed the backport-12 Automatically create a backport for the stable-12 branch label Sep 28, 2026
@felixfontein felixfontein removed the backport-13 Automatically create a backport for the stable-13 branch label Oct 6, 2026
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Oct 6, 2026
@felixfontein

felixfontein commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator Author

@ansibullbot ansibullbot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. lookup lookup plugin needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging tests tests unit tests/unit WIP Work in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@felixfontein @ansibullbot @kingtong