fix(release): publish:git after anti-tamper check (projen#1752)

The anti-tamper check should occur before git publication to prevent the tag and commit from occurring in case the check fails. Closes projen#1751 --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
FitX · Apr 11, 2022 · 5f1dd9b · 5f1dd9b
1 parent eed1e75
commit 5f1dd9b
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 33 deletions.
diff --git a/docs/api/API.md b/docs/api/API.md
@@ -8819,6 +8819,7 @@ Name | Type | Description
 **jsiiReleaseVersion**⚠️ | <code>string</code> | <span></span>
 **publibVersion**🔹 | <code>string</code> | <span></span>
 **condition**?🔹 | <code>string</code> | __*Optional*__
+*static* **PUBLISH_GIT_TASK_NAME**🔹 | <code>string</code> | <span></span>
 
 ### Methods
 
@@ -9039,6 +9040,7 @@ Name | Type | Description
 **artifactsDirectory**🔹 | <code>string</code> | Location of build artifacts.
 **branches**🔹 | <code>Array<string></code> | Retrieve all release branch names.
 **publisher**🔹 | <code>[release.Publisher](#projen-release-publisher)</code> | Package publisher.
+*static* **ANTI_TAMPER_CMD**🔹 | <code>string</code> | <span></span>
 
 ### Methods
 

diff --git a/src/release/publisher.ts b/src/release/publisher.ts
@@ -111,6 +111,8 @@ export interface PublisherOptions {
  * Under the hood, it uses https://github.com/aws/publib
  */
 export class Publisher extends Component {
+  public static readonly PUBLISH_GIT_TASK_NAME = "publish:git";
+
   public readonly buildJobId: string;
   public readonly artifactName: string;
   public readonly publibVersion: string;
@@ -197,8 +199,8 @@ export class Publisher extends Component {
 
     const taskName =
       gitBranch === "main" || gitBranch === "master"
-        ? "publish:git"
-        : `publish:git:${gitBranch}`;
+        ? Publisher.PUBLISH_GIT_TASK_NAME
+        : `${Publisher.PUBLISH_GIT_TASK_NAME}:${gitBranch}`;
 
     const publishTask = this.project.addTask(taskName, {
       description:

diff --git a/src/release/release.ts b/src/release/release.ts
@@ -226,6 +226,8 @@ export interface ReleaseOptions extends ReleaseProjectOptions {
  * By default, no branches are released. To add branches, call `addBranch()`.
  */
 export class Release extends Component {
+  public static readonly ANTI_TAMPER_CMD =
+    "git diff --ignore-space-at-eol --exit-code";
   /**
    * Returns the `Release` component of a project or `undefined` if the project
    * does not have a Release component.
@@ -508,6 +510,10 @@ export class Release extends Component {
     releaseTask.spawn(this.buildTask);
     releaseTask.spawn(this.version.unbumpTask);
 
+    // anti-tamper check (fails if there were changes to committed files)
+    // this will identify any non-committed files generated during build (e.g. test snapshots)
+    releaseTask.exec(Release.ANTI_TAMPER_CMD);
+
     if (this.releaseTrigger.isManual) {
       const publishTask = this.publisher.publishToGit({
         changelogFile: path.posix.join(
@@ -530,10 +536,6 @@ export class Release extends Component {
       releaseTask.spawn(publishTask);
     }
 
-    // anti-tamper check (fails if there were changes to committed files)
-    // this will identify any non-committed files generated during build (e.g. test snapshots)
-    releaseTask.exec("git diff --ignore-space-at-eol --exit-code");
-
     const postBuildSteps = [...this.postBuildSteps];
 
     // check if new commits were pushed to the repo while we were building.

diff --git a/test/release/release.test.ts b/test/release/release.test.ts
@@ -1,6 +1,6 @@
 import * as YAML from "yaml";
 import { JobPermission } from "../../src/github/workflows-model";
-import { Release, ReleaseTrigger } from "../../src/release";
+import { Publisher, Release, ReleaseTrigger } from "../../src/release";
 import { synthSnapshot, TestProject } from "../util";
 
 test("minimal", () => {
@@ -222,6 +222,58 @@ test("releaseSchedule schedules releases", () => {
   });
 });
 
+test("manual release publish happens after anti-tamper check", () => {
+  // GIVEN
+  const project = new TestProject();
+
+  // WHEN
+  new Release(project, {
+    task: project.buildTask,
+    versionFile: "version.json",
+    branch: "main",
+    releaseTrigger: ReleaseTrigger.manual(),
+    artifactsDirectory: "dist",
+  });
+
+  // THEN
+  const outdir = synthSnapshot(project);
+  const steps: Object[] = outdir[".projen/tasks.json"].tasks.release.steps;
+  const antiTamperStepIndex = steps.findIndex(
+    (obj: any) => obj.exec === Release.ANTI_TAMPER_CMD
+  );
+  const publishGitStepIndex = steps.findIndex(
+    (obj: any) => obj.spawn === Publisher.PUBLISH_GIT_TASK_NAME
+  );
+  expect(publishGitStepIndex).toBeGreaterThan(antiTamperStepIndex);
+});
+
+test("manual release with custom git-push", () => {
+  // GIVEN
+  const project = new TestProject();
+  new Release(project, {
+    task: project.buildTask,
+    versionFile: "version.json",
+    branch: "main",
+    releaseTrigger: ReleaseTrigger.manual({
+      gitPushCommand: "git push --follow-tags -o ci.skip origin main",
+    }),
+    publishTasks: true, // to increase coverage
+    artifactsDirectory: "dist",
+  });
+
+  // THEN
+  const outdir = synthSnapshot(project);
+  const steps =
+    outdir[".projen/tasks.json"].tasks[Publisher.PUBLISH_GIT_TASK_NAME].steps;
+  expect(steps).toEqual(
+    expect.arrayContaining([
+      expect.objectContaining({
+        exec: "git push --follow-tags -o ci.skip origin main",
+      }),
+    ])
+  );
+});
+
 test("addJobs() can be used to add arbitrary jobs to the release workflows", () => {
   // GIVEN
   const project = new TestProject();
@@ -507,32 +559,6 @@ test("can be modified with escape hatches", () => {
   expect(outdir).toMatchSnapshot();
 });
 
-test("manual release with custom git-push", () => {
-  // GIVEN
-  const project = new TestProject();
-  new Release(project, {
-    task: project.buildTask,
-    versionFile: "version.json",
-    branch: "main",
-    releaseTrigger: ReleaseTrigger.manual({
-      gitPushCommand: "git push --follow-tags -o ci.skip origin main",
-    }),
-    publishTasks: true, // to increase coverage
-    artifactsDirectory: "dist",
-  });
-
-  // THEN
-  const outdir = synthSnapshot(project);
-  const steps = outdir[".projen/tasks.json"].tasks["publish:git"].steps;
-  expect(steps).toEqual(
-    expect.arrayContaining([
-      expect.objectContaining({
-        exec: "git push --follow-tags -o ci.skip origin main",
-      }),
-    ])
-  );
-});
-
 test("publisher can use custom github runner", () => {
   // GIVEN
   const project = new TestProject();