automation: substitute (env-) variables in jobs

[rethab]

[psiinon]

Thanks for taking this on! Have you checked that the rest of the code (including the UI) copes with env vars? I know in some cases we were checking that elements were valid, ie that an integer really was an int. Where env vars are already supported we have had to disable those checks in some places, eg https://github.com/zaproxy/zap-extensions/blob/main/addOns/automation/src/main/java/org/zaproxy/addon/automation/jobs/RequestorJob.java#L102 I was going to add an isEnvVar(str) method but havnt got around to it ;)

[psiinon]

@rethab from the build failure logs Run './gradlew :addOns:automation:spotlessApply' to fix these violations.

[rethab]

Thanks for the hints so far @psiinon & @thc202 .

I guess this turns out to be not quite as easy as I thought/hoped, but I'm not giving up just yet. Could you please give me some hints though? Where do you think the substitution should take place?

And also, how should I make sure that when the automation plan is written back to a file that the environment variables are not substituted in that case? My thinking was that perhaps the "raw" job data could be retained and then before writing out the parameters, we would make sure that those are in their original state again. But not sure how feasible that is :)

Lastly, I would curious to learn more about the distinction between parameters and job data. When is which used? What makes something a parameter (e.g. theme for report job) vs. job data (e.g. risks for report job)?

[rethab]

@psiinon @thc202 would you have some time to take another look?

[psiinon]

@rethab there are failing tests

[rethab]

Thanks for the hints so far @psiinon & @thc202 .

I guess this turns out to be not quite as easy as I thought/hoped, but I'm not giving up just yet. Could you please give me some hints though? Where do you think the substitution should take place?

And also, how should I make sure that when the automation plan is written back to a file that the environment variables are not substituted in that case? My thinking was that perhaps the "raw" job data could be retained and then before writing out the parameters, we would make sure that those are in their original state again. But not sure how feasible that is :)

Lastly, I would curious to learn more about the distinction between parameters and job data. When is which used? What makes something a parameter (e.g. theme for report job) vs. job data (e.g. risks for report job)?

@psiinon if you have a moment, could you please respond to this?

[psiinon]

Thanks for the hints so far @psiinon & @thc202 . I guess this turns out to be not quite as easy as I thought/hoped, but I'm not giving up just yet. Could you please give me some hints though? Where do you think the substitution should take place? And also, how should I make sure that when the automation plan is written back to a file that the environment variables are not substituted in that case? My thinking was that perhaps the "raw" job data could be retained and then before writing out the parameters, we would make sure that those are in their original state again. But not sure how feasible that is :) Lastly, I would curious to learn more about the distinction between parameters and job data. When is which used? What makes something a parameter (e.g. theme for report job) vs. job data (e.g. risks for report job)?

@psiinon if you have a moment, could you please respond to this?

OK, so the split is probably not as clear as it should be, but parameters are always single values - they cannot have structure. Job data can have structure, eg a hierarchic structure, multiple values etc

[psiinon]

I think the best option is to leave the env vars in the data for as long as possible, so basically just applying them when they are applied to ZAP. This should be how they are applied now, so look to see where they are currently done, e.g. https://github.com/zaproxy/zap-extensions/search?q=replaceVars&type=code

Note that we already apply them here for Strings: https://github.com/zaproxy/zap-extensions/blob/main/addOns/automation/src/main/java/org/zaproxy/addon/automation/jobs/JobUtils.java#L374 In theory we could apply them for other objects but we'd need tests for everywhere they could be used just to make sure its ok.

[thc202]

@rethab ping.

[rethab]

@rethab ping.

@thc202 @psiinon

I no longer have the time or motivation to work on this. I'm going to close this PR. I am sorry that I wasted your time on this.

[thc202]

No time wasted, good thoughts about the issue either way. Thank you!