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

Interpolate credentials #3153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarkBarbieri
Copy link

Interpolate credentials to allow the use and expansion of liquid tags in inside credentials

Typical use case is defining the body for an EmailAgent using credentials. This ensures any liquid formatting inside the credential is correctly interpolated.

@dsander, you mentioned a different use case in this post #2121 (comment)

I think this is worthwhile, although you may have a better implementation.

@knu
Copy link
Member

knu commented Nov 3, 2023

Hmm, I think this will allow for recursive interpolation and break Liquid's security model. (by putting a {% credential %} tag inside of a credential value)

We can probably introduce global variables to replace credentials where each value is defined in JSON.

@knu
Copy link
Member

knu commented Nov 3, 2023

How about adding a "data type" (string or JSON) column to credentials? /cc @dsander

@virtadpt
Copy link
Collaborator

virtadpt commented Nov 3, 2023

What about building the message body piecewise?

...
"body": "{% credential foo %} lkjl;kj  lk;jl;kjl  {% credential bar %} {% credential baz %} l;kajsdl;kfjds...."
...

It would be more flexible and less prone to accidental recursion.

@MarkBarbieri
Copy link
Author

What about building the message body piecewise?


...

"body": "{% credential foo %} lkjl;kj  lk;jl;kjl  {% credential bar %} {% credential baz %} l;kajsdl;kfjds...."

...

It would be more flexible and less prone to accidental recursion.

That would be ok if there was only one or two tags, but if there are multiple then it will get messy quickly. For example, a custom email message body that displays multiple tags

@MarkBarbieri
Copy link
Author

Hmm, I think this will allow for recursive interpolation and break Liquid's security model. (by putting a {% credential %} tag inside of a credential value)

I suppose that could be easily tested to see if it is the case.

@dsander
Copy link
Collaborator

dsander commented Nov 7, 2023

Hmm, I think this will allow for recursive interpolation and break Liquid's security model. (by putting a {% credential %} tag inside of a credential value)

Don't we - in theory - have that problem in more places? I am not so concerned about it because the user would be creating the recursive interpolation by themselves.

How about adding a "data type" (string or JSON) column to credentials? /cc @dsander

Maybe we are thinking about the same here. What I did not consider when I made the initial suggestion is backwards compatibility. If we would interpolate liquid in all credentials the result of secret{{ test }} would change and break existing setups. Maybe a new credential mode could enable the interplation?

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.

None yet

4 participants