Skip to content

Comments

Fix script injection issues#987

Merged
gpeng merged 1 commit intomainfrom
DTOSS-12207-fix-script-injection-issues
Feb 11, 2026
Merged

Fix script injection issues#987
gpeng merged 1 commit intomainfrom
DTOSS-12207-fix-script-injection-issues

Conversation

@gpeng
Copy link
Contributor

@gpeng gpeng commented Feb 4, 2026

Description

We have 'a lot' of security alerts regarding string interpolation in run blocks in GitHub actions. I'm not sure they're particularly exploitable and I don't think we're running some of them (something else to review another time) but I've applied the suggested mitigation and added a linter rule to check for future 'violations'.

Also reordered the linting as I was getting prettier failures due to some local files and that was stopping the later linting from running.

If this gets approved/merged and I'll update the repo template.

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-12207

Copy link
Contributor

@MatMoore MatMoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good, but I would rather not commit the python script (and it also creates an additional Sonar issue)

@malcolmbaig malcolmbaig changed the title DTOSS 12207 fix script injection issues Fix script injection issues Feb 9, 2026
This resolves Github security alerts for the workflows. Interpolating
user input in the run block allows script injection. Moved to env as
recommended in the alerts.
@gpeng gpeng force-pushed the DTOSS-12207-fix-script-injection-issues branch from e688f99 to 0dcf2fd Compare February 9, 2026 11:04
@gpeng
Copy link
Contributor Author

gpeng commented Feb 9, 2026

Thanks @MatMoore. I've removed the linter script commit.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 9, 2026

@gpeng gpeng merged commit 30ad2f0 into main Feb 11, 2026
14 checks passed
@gpeng gpeng deleted the DTOSS-12207-fix-script-injection-issues branch February 11, 2026 11:51
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