Skip to content

sanitize preview when handling secret parameters #1036

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fdrab
Copy link
Contributor

@fdrab fdrab commented Jul 22, 2025

re-introducing sanitization of preview when handling secrets - issue #1005

@nzlosh nzlosh requested a review from a team July 25, 2025 11:35
@nzlosh
Copy link
Contributor

nzlosh commented Jul 25, 2025

Should this PR have a changelog entry?

@fdrab
Copy link
Contributor Author

fdrab commented Jul 31, 2025

I'll add it.

@cognifloyd
Copy link
Member

cognifloyd commented Aug 12, 2025

With this change the only way to check i typed a secret correctly will be to use the visibility button on each secret field added in #1035, right?

Will this affect secret fields with an object or array? Today, preview is the only way I can validate that I typed or pasted structured data correctly.

@fdrab
Copy link
Contributor Author

fdrab commented Aug 13, 2025

With this change the only way to check i typed a secret correctly will be to use the visibility button on each secret field added in #1035, right?

Will this affect secret fields with an object or array? Today, preview is the only way I can validate that I typed or pasted structured data correctly.

Answer to both questions is "Yes":
image

@cognifloyd
Copy link
Member

Does the preview make secret_object look like it is a string? If so, could it be masked as something like {****} instead of "****"?

@fdrab
Copy link
Contributor Author

fdrab commented Aug 13, 2025

like this?
image

@fdrab
Copy link
Contributor Author

fdrab commented Aug 13, 2025

If the above is not satisfactory, perhaps I can, instead of replacing all chars with * just generate random alphanumeric strings to replace the originals with? Would that be better?

@cognifloyd
Copy link
Member

That is better. Something nicer would be great, but it's probably not required to get this in. That would be a great follow-up PR though.

I think we need the hide/show secrets button merged before this goes in.

@fdrab
Copy link
Contributor Author

fdrab commented Aug 16, 2025

do not merge this pls, this needs adjustment

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