Merge pull request #66 from NVIDIA-NeMo/harden-frontend-build-verification

fix(viewer): make the frontend build hermetic and actually verify dist/

Author: rdasilveiracabral

.github/workflows/ci.yml

@@ -62,26 +62,76 @@ jobs:
           path: dist/
           retention-days: 7
 
+  # Proves the committed viewer bundle is exactly what building the committed
+  # source produces. dist/ is 424 KB of minified JS that no reviewer can read,
+  # so without this the bundle is an unreviewable trust input that any
+  # contributor could poison independently of the TypeScript they submit. With
+  # it, dist/ is a derived value CI re-derives from scratch on every PR, and the
+  # only way into the viewer is through a reviewable source diff.
+  #
+  # This job is only meaningful if it cannot be skipped: for `pull_request`,
+  # GitHub runs the workflow as written in the PR, so a PR may delete this job.
+  # It must be a REQUIRED status check on main — otherwise a PR that removes it
+  # merges green. See also CODEOWNERS on package*.json: `npm run build` executes
+  # vite.config.ts and every plugin in node_modules, which `--ignore-scripts`
+  # does not cover, so a lockfile change is arbitrary code execution here.
   frontend-build:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
       - uses: actions/checkout@v4
       - name: Setup Node
         uses: actions/setup-node@v4
         with:
-          node-version: "20"
-      - name: Build the trace viewer
+          # Exact pin: this job's output IS the security property, so the
+          # toolchain producing it should not float. Output was verified
+          # byte-identical on 20.19.5 and 22.20.0, so a bump is safe.
+          node-version: "20.19.5"
+      - name: Rebuild the viewer bundle from source
         working-directory: src/nooa/viewer/frontend-react
         run: |
+          set -euo pipefail
           npm ci --ignore-scripts
+          # Build into an EMPTY dist/. Two reasons, both load-bearing:
+          #  1. A file that is committed but that the build does not produce
+          #     (i.e. smuggled in) then shows up as deleted rather than silently
+          #     surviving untouched.
+          #  2. The build must not see the artifact it is being checked against.
+          #     src/index.css carries `@source not '../dist'` so Tailwind skips
+          #     it; wiping dist/ means the check holds even if that regresses.
+          rm -rf dist
           npm run build
-      - name: Fail if dist/ is stale
+          # Audit trail: the hashes CI derived, readable in the job log.
+          sha256sum dist/index.html dist/assets/*
+      - name: Fail if committed dist/ is not exactly what the build produced
         run: |
-          if ! git diff --quiet src/nooa/viewer/frontend-react/dist/; then
-            echo "ERROR: frontend-react/dist/ is stale. Run 'npm run build' and commit dist/."
-            git diff --stat src/nooa/viewer/frontend-react/dist/
+          set -euo pipefail
+          DIST=src/nooa/viewer/frontend-react/dist
+          # Guard against a vacuous pass. `git status --porcelain <path>` warns
+          # on stderr and prints NOTHING when the path does not resolve, which
+          # reads as success — so a wrong path or a relocated dist/ would report
+          # a clean build having verified nothing. Same failure mode the
+          # gitleaks job guards against below. Assert we are looking at real,
+          # tracked files before trusting an empty result.
+          test -d "$DIST" \
+            || { echo "::error::$DIST does not exist — the build did not produce it"; exit 1; }
+          test "$(git ls-files -- "$DIST" | wc -l)" -gt 0 \
+            || { echo "::error::no tracked files under $DIST — the check would pass vacuously"; exit 1; }
+          # `git status --porcelain`, not `git diff`: git diff only reports
+          # tracked-and-modified files, so it passes a PR that ADDS a file to
+          # dist/. Porcelain reports ' M' modified, '??' produced-but-uncommitted
+          # and ' D'/'AD' committed-but-not-produced — all three are failures.
+          CHANGES=$(git status --porcelain -- "$DIST")
+          if [ -n "$CHANGES" ]; then
+            echo "::error::dist/ does not match a clean rebuild of the committed source."
+            echo "$CHANGES"
+            echo
+            echo "If you changed the viewer UI, rebuild and commit the result:"
+            echo "  cd src/nooa/viewer/frontend-react && npm ci --ignore-scripts && rm -rf dist && npm run build"
             exit 1
           fi
+          echo "OK — committed dist/ is reproducible from source."
 
   secret-scan:
     runs-on: ubuntu-latest

src/nooa/viewer/frontend-react/dist/assets/index-BN0Wf3Y-.css

@@ -4,8 +4,8 @@
     <meta charset="UTF-8" />
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <title>NVIDIA OO Agents Viewer</title>
-    <script type="module" crossorigin src="/assets/index-Bf6CiCw-.js"></script>
-    <link rel="stylesheet" crossorigin href="/assets/index-BN0Wf3Y-.css">
+    <script type="module" crossorigin src="/assets/index-DwrEQzTi.js"></script>
+    <link rel="stylesheet" crossorigin href="/assets/index-DKBq4F_b.css">
   </head>
   <body>
     <div id="root"></div>

src/nooa/viewer/frontend-react/dist/assets/index-DKBq4F_b.css

@@ -1,6 +1,21 @@
 @import 'tailwindcss';
 @import 'highlight.js/styles/vs2015.css';
 
+/* Exclude the build output from Tailwind's automatic source detection.
+ *
+ * dist/ is committed, so Tailwind's project scan would otherwise read the
+ * PREVIOUS bundle and treat words inside it as class-name candidates. That
+ * makes the build self-referential: `npm run build` over a populated dist/
+ * emits utilities (.static, .table, .resize) that a clean build does not, so
+ * the committed state is a fixed point rather than a function of src/.
+ *
+ * Two consequences, both bad. CI's rebuild-and-compare check only passes
+ * because it builds on top of the artifact it is verifying, and a contributor
+ * can influence the emitted CSS by planting strings in dist/ instead of in
+ * reviewable source. This directive makes the build a pure function of src/.
+ */
+@source not '../dist';
+
 body {
   @apply bg-gray-950 text-gray-200 antialiased;
   font-family: ui-sans-serif, system-ui, sans-serif;