Skip to content

feat(controls): Add unsafe variable expansion control for user-filled predefined variables#94

Merged
Joseph94m merged 1 commit intomainfrom
feat/control-unsafe-variable-expansion
Mar 4, 2026
Merged

feat(controls): Add unsafe variable expansion control for user-filled predefined variables#94
Joseph94m merged 1 commit intomainfrom
feat/control-unsafe-variable-expansion

Conversation

@Joseph94m
Copy link
Collaborator

Implementation notes: Issue #89 – pipelineMustNotUseUnsafeVariableExpansion

Closes #89

What we implemented (vs proposed)

Aligned with the issue

  • Control name: pipelineMustNotUseUnsafeVariableExpansion.
  • Scope: Scans script:, before_script:, and after_script: (per-job and global) using merged config from PipelineOriginData.
  • Config: enabled (required), dangerousVariables (list), allowedPatterns (list of regexes). Default variable list matches the issue (e.g. CI_MERGE_REQUEST_TITLE, CI_COMMIT_MESSAGE, CI_COMMIT_REF_NAME, …).
  • Compliance: 0% if any finding, 100% otherwise.
  • False positives: allowedPatterns supported; a script line matching any pattern is not reported even if it contains a dangerous variable.
  • Files touched: New control in control/controlGitlabPipelineVariableInjection.go; wiring in control/types.go, control/task.go, configuration/plumberconfig.go, .plumber.yaml, cmd/analyze.go; MR summary in control/mrcomment.go; script line extraction in gitlab/utilsCI.go. Docs: README.md, COMPONENT_README.md.

Divergences and decisions

Topic Proposed Implemented
What to flag Flag dangerous variables when used unquoted, in backtick substitution, or in eval. Issue suggested an optional “flag any usage” as a first step. We only flag when a dangerous variable appears on a line that is in a shell re-interpretation context: eval, sh -c, bash -c, dash -c, zsh -c, ksh -c, envsubst … | sh/bash, xargs sh/bash, source, or . (dot source). Plain usage such as echo $CI_COMMIT_BRANCH or echo "$CI_COMMIT_MESSAGE" is not reported: the shell does not re-parse the expanded value, so there is no injection there.
Severity Contextual severity: e.g. error for eval/backticks, warning for unquoted, note for quoted. No per-finding severity. All findings are treated the same; control-level severity is not exposed in this control’s config.
Detection patterns Explicitly: unquoted, eval, backticks. Re-interpretation only (see above). We added patterns for source, . , `envsubst
allowedPatterns “Allow an allowedPatterns list in config for teams that have vetted specific usage patterns.” Implemented as line-level regex: if the entire script line matches any allowedPatterns entry, the line is not reported.

Documented limitation

  • Only direct variable names are matched. Indirect aliasing (e.g. variables: { B: $CI_COMMIT_BRANCH } and later sh -c $B) is not tracked.

@Joseph94m Joseph94m force-pushed the feat/control-unsafe-variable-expansion branch from df3de7b to ba1bc13 Compare March 4, 2026 16:11
@Joseph94m Joseph94m merged commit 0f7ec72 into main Mar 4, 2026
7 checks passed
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.

[FEAT] New control: pipelineMustNotUseUnsafeVariableExpansion: detect CI variable injection risks in scripts

1 participant