Skip to content

fix(ng-dev/release): escape double quotes in snapshot commit message#3526

Merged
alan-agius4 merged 1 commit intoangular:mainfrom
josephperrott:fix-message
Mar 12, 2026
Merged

fix(ng-dev/release): escape double quotes in snapshot commit message#3526
alan-agius4 merged 1 commit intoangular:mainfrom
josephperrott:fix-message

Conversation

@josephperrott
Copy link
Copy Markdown
Member

When creating a snapshot commit, the snapshot message might contain double quotes which need to be properly escaped when passed to the git command line. This fixes an issue where the git commit -m command fails or produces incorrectly formatted messages if the snapshot message contains quotes.

When creating a snapshot commit, the snapshot message might contain double quotes which need to be properly escaped when passed to the git command line. This fixes an issue where the `git commit -m` command fails or produces incorrectly formatted messages if the snapshot message contains quotes.
@josephperrott josephperrott added the action: merge The PR is ready for merge by the caretaker label Mar 12, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to fix an issue with double quotes in snapshot commit messages. However, the proposed change incorrectly adds shell-style escaping for a command that doesn't use a shell. This would result in malformed commit messages. I've added a comment with a suggestion to revert to the original, correct logic.

Comment on lines +164 to +173
this.git.run(
[
'commit',
'--author',
this.commitAuthor,
'-m',
`"${this.snapshotCommitMessage.replace(/"/g, '\\"')}"`,
],
{cwd: tmpRepoDir},
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The added escaping of double quotes and wrapping the message in quotes is not necessary and is likely to cause issues. The git.run method uses child_process.spawnSync which, when given an array of arguments, passes them directly to the git executable without an intermediate shell. This means shell-style quoting is not needed.

With this change, if snapshotCommitMessage is feat: handle "quotes", the actual commit message will become "feat: handle \"quotes\"", including the outer quotes and the backslash, which is not the intended behavior.

The original implementation was correct. Please revert this part of the change.

        this.git.run(['commit', '--author', this.commitAuthor, '-m', this.snapshotCommitMessage], {
          cwd: tmpRepoDir,
        });

@alan-agius4 alan-agius4 merged commit 03f14dc into angular:main Mar 12, 2026
13 checks passed
@alan-agius4
Copy link
Copy Markdown
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants