Skip to content

Prevent retrieval of masked group CI variables

Lee Tickett requested to merge 29674-prevent-retrieving-masked-variables into master

What does this MR do and why?

Related to #29674 (closed) and !78940 (closed)

In today, we can have the values protected & avoid disclosure during pipeline execution. If someone has admin/maintainer access they are able to access reveal these variables. We should have an option to make the value permanent hidden so that, anyone having admin/maintainer access to that project, will not be able to see the value.

This MR starts to attack this by preventing retrieval of masked group variables.

Screenshots or screen recordings

video

GET'ing all group variables reveals unmasked variable values, but not masked ones:

image

GET'ing a single masked group variable does not reveal it's value:

image

Trying to PUT a masked variable to unmasked to reveal it's value returns an error:

image

PUT'ing a new value whilst also "unmasking" a variable dot not reveal the old value, but does reveal the new value:

image

How to set up and validate locally

1. Create at least 1 masked and 1 unmasked group variable (I used http://gdk.test:3000/groups/flightjs/-/settings/ci_cd)
2. Ensure you can reveal the masked variable through the UI and via the API: 
   - http://gdk.test:3000/api/v4/groups/26/variables
   - http://gdk.test:3000/api/v4/groups/26/variables/masked_variable_key
3. Feature.enable(:prevent_masked_variable_retrieval)
4 Ensure you can NOT reveal the masked variable through the UI or via the API (you should just see nothing/blank/null): 
   - http://gdk.test:3000/api/v4/groups/26/variables
   - http://gdk.test:3000/api/v4/groups/26/variables/masked_variable_key
5. Ensure you cannot change the masked status of the masked variable without resetting the value (again, both through the UI and API)
   - http://gdk.test:3000/api/v4/groups/26/variables/masked_variable_key (PUT { "masked": false } should error)
   - http://gdk.test:3000/api/v4/groups/26/variables/masked_variable_key (PUT { "masked": false, "value": "new-value" } should succeed)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #29674 (closed)

Edited by Lee Tickett

Merge request reports