jdalrymple / gitbeaker

🦊🧪 A comprehensive and typed Gitlab SDK for Node.js, Browsers, Deno and CLI
Other
1.55k stars 294 forks source link

PipelineTriggerTokens endpoint incorrectly handles variables via reformatObjectOptions #3574

Closed nickjm closed 5 months ago

nickjm commented 5 months ago

Description

We create new pipelines or trigger downstream pipelines contextually depending on whether we're running locally or in an existing CI pipeline. One of the variables passed to the pipeline via GitBeaker's Pipeline.create and PipelineTriggers.trigger endpoints is a valid JSON string (not necessarily URL safe). If the JSON string contains an = it breaks the triggers endpoint specifically because reformatObjectOptions splits on = here without doing any pre-processing of the string beforehand. In our case the way it breaks is in the CI pipeline the JSON is just abruptly cut off where the = would have been.

Steps to reproduce

await api.PipelineTriggerTokens.trigger(<project_id>, 'main', <job-token>, {variables: {CONFIG_JSON: '{"special-var": "=5"}'}})

Expected behaviour

variable with JSON is properly encoded, just as it is when using the Pipelines.create endpoint, such that the CI Pipeline sees

$ echo "${CONFIG_JSON}"
{"special-var": "=5"}

Actual behaviour

JSON is split up based on the presence of a =.

$ echo "${CONFIG_JSON}"
{"special-var": "

Possible fixes

I'm not sure why reformatObjectOptions is needed in the first place, but I do recognize that GitLab's API is different across these two endpoints, and the documentation was sparse on the formatting expected for variables in these different endpoints. However, I think it's reasonable to expect that as long as you format the variables object correctly, the create and trigger endpoints should behave roughly the same and pre-process variables in a consistent manner.

Checklist

jdalrymple commented 5 months ago

The reformatObjectOptions was introduced back here, as a way to handle the weird formatting Gitlab went with for nested object payloads. This is most visible in the discussions position arguments and like youve seen the variables argument for pipeline tokens. Basically, this functional is used to serialise the nested objects since the contents are not send in a body variable, but as a formData key/value pair. Why they choose to do it that way is beyond me haha caused me enough headaches.

In this case, its a bit of an edge case since im ideally parsing the output of qs which is a clean query parameter in the form of key=value. In this case its key==value. Ill tweak the split

jdalrymple commented 2 months ago

:rocket: Issue was released in 40.1.0 :rocket: