Sequence marked "final" can override action parameters

[cjelger]

When a web action sequence is marked final, it is still possible to override the parameters of its internal actions.

To reproduce: create a final web action sequence using some action with a bound parameter. Check that it is possible to override that parameter when calling the web action.

@rabbah as discussed on Slack.

[cjelger]

@rabbah It's been a while since we had that discussion, is this something that could be implemented? It could become a serious DDOS issue, for example if one can redirect actions that are supposed to hit some backend services (S3, ecommerce system, caching system, whatever configured in some action parameters) to a target victim. Would it be complicated to fix this?

[rabbah]

@cjelger thanks for pinging on this. The issue here is that sequences aren't allowed parameters (for historical reasons that are no longer applicable). There is a long discussion about sequence parameters - should they be local (visible only to the first action in the sequence) or global (scoped to all actions in the sequence). If we make them local at least we can allow the sequence to protect the parameters of the first action.

In short, yes we should fix this. Are you interested in sending a contribution/patch? I'm happy to help.

[cjelger]

Thx @rabbah for your answer. I'm not sure I correctly understand: will we have to choose between having global OR final parameters, but not both? I think no longer having global parameters for sequences may break a lot of existing code that now rely on that ... can you please clarify?

I'd be interested to work on that but I've never been involved in OW development, I'm mostly very active on the user side. ;-) Also I'm unfortunately not a Scala developer, but I might give it a try.

[rabbah]

Sequences aren’t allowed to carry parameters at all today iirc. So there’s no backward compatibility to really be concerned with. I’ll link the issue on parameter discussion here.

[cjelger]

Actually in our project we deploy actions in shared packages, and customers create package bindings and add their parameters to the bindings, and these parameters are then "inherited" by sequences. And we'd like to avoid that these parameters can be overriden by request parameters. I'll check again what happens if I try to define the sequences (web actions) as final.

[cjelger]

I checked and could override the parameters defined in the package bindings, even if I define the sequence web action as final. I also tried adding the final annotation to the package binding but it doesn't have any effect. I have no idea about OW internal implementation details, but maybe one option could be that one could define the package binding as final, and would basically avoid that the binding parameters get overridden by request parameters. WDYT? Does this sound feasible?

[rabbah]

The defect is that sequences have a parameter set but it’s empty. When the final override check happens it’s against this set, not the set of parameters that are attached to the first action of the sequence.

Package/binding params are no different than any parameter attached to the first action in the sequence in this regard.

maybe this is what you’re suggesting: a first step is to change the final check on a sequence action so that it checks its first component instead?

[rabbah]

Recording an additional thought/proposal on this - the following might be a better approach: extend the applicability of final parameters to all actions. Currently the check is on the web action invoke path only, and applies to the top most action.

[MichaelGoberling]

@rabbah @cjelger

Re-opening this discussion as I've recently run into a use case that would benefit from this feature being added. (Our desired use case is basically to create a bindable validator action with an allow-list that will be used at the start of sequences, the allow-list should be immutable)

I think applying the final annotation to all actions would be useful. In my experience, it's a common pattern to create a set of non-web actions and then expose them via a web sequence.

As for the final annotation not applying to sequences - A possible path forward would be to make the final annotation check a little more intelligent by inspecting each package and each action's parameter list to see if a passed parameter is present. If the final annotation is included for the sequence, we check everything. If the final annotation is not included, then we respect the final annotation on each package and action, if present.

[bdoyle0182]

Hi @MichaelGoberling, the project is pretty short staffed right now; tbh I don't think anything related to sequences has been touched in years. If Rodric thinks this is something that makes sense to change and you would be up to contributing, I would be happy to help review.