Skip to content

When updating a sensitive variable, the value needs to be null. This … - #261

Draft
mcasperson wants to merge 11 commits into
mainfrom
mattc/allow_null_variable_value
Draft

When updating a sensitive variable, the value needs to be null. This …#261
mcasperson wants to merge 11 commits into
mainfrom
mattc/allow_null_variable_value

Conversation

@mcasperson

Copy link
Copy Markdown
Contributor

…changes allows for a nil string for the value.

@NickJosevski

Copy link
Copy Markdown
Contributor

I measured this against a live 2026.x server before picking up the change, and the premise doesn't hold — sending null makes no difference. A runbook printed ${#VALUE} after each write so masking couldn't hide the result; starting value 18 characters:

Id sent Value sent Resulting length
yes "" 18 — preserved
yes null 18 — preserved
no "" 0 — wiped
no null 0 — wiped

The Id is what preserves the secret, not the null. So changing Value to *string would break every caller of Variable.Value without preventing the wipe.

There is a real bug underneath though, filed as #449: writing a sensitive variable with no value and no Id silently destroys the stored secret. #450 fixes that by rejecting the payload, keeping Value as string.

The GitHub credential half of this branch has also landed independently as credentials.GitHubApp.

Leaving this open for you @mcasperson — happy to be told I've missed a server version or a code path where the null does matter.

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.

2 participants