From 9709471485c93c0d01ce1b030a8d82e20d761588 Mon Sep 17 00:00:00 2001
From: devitway <devitway@gmail.com>
Date: Fri, 20 Mar 2026 22:14:16 +0000
Subject: [PATCH] fix: address code review findings

- Pin slsa-github-generator and codeql-action by SHA (not tag)
- Replace anonymous tuple with GroupedActivity struct for readability
- Replace unwrap() with if-let for safety
- Add warning message on attestation failure instead of silent || true
- Fix clippy: map_or -> is_some_and
---
 .github/workflows/release.yml     |  6 +--
 .github/workflows/scorecard.yml   |  2 +-
 nora-registry/src/ui/templates.rs | 63 +++++++++++++++++++------------
 3 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index 3929fef..d81f8c2 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -144,7 +144,7 @@ jobs:
 
       - name: Smoke test — verify alpine image starts and responds
         run: |
-          docker rm -f nora-smoke 2>/dev/null || true
+          docker rm -f nora-smoke 2>/dev/null || echo "WARNING: attestation failed, continuing without provenance"
           docker run --rm -d --name nora-smoke -p 5555:4000 -e NORA_HOST=0.0.0.0 \
             ${{ env.NORA }}/${{ env.IMAGE_NAME }}:latest
           for i in $(seq 1 10); do
@@ -226,7 +226,7 @@ jobs:
           cat nora-linux-amd64.sha256
 
       - name: Generate SLSA provenance
-        uses: slsa-framework/slsa-github-generator/.github/actions/generate-builder@v2.1.0
+        uses: slsa-framework/slsa-github-generator/.github/actions/generate-builder@f7dd8c54c2067bafc12ca7a55595d5ee9b75204a # v2.1.0
         id: provenance-generate
         continue-on-error: true
 
@@ -234,7 +234,7 @@ jobs:
         if: always()
         run: |
           # Generate provenance using gh attestation (built-in GitHub feature)
-          gh attestation create ./nora-linux-amd64             --repo ${{ github.repository }}             --signer-workflow ${{ github.server_url }}/${{ github.repository }}/.github/workflows/release.yml 2>/dev/null || true
+          gh attestation create ./nora-linux-amd64             --repo ${{ github.repository }}             --signer-workflow ${{ github.server_url }}/${{ github.repository }}/.github/workflows/release.yml 2>/dev/null || echo "WARNING: attestation failed, continuing without provenance"
           # Also create a simple provenance file for scorecard
           cat > nora-v${{ github.ref_name }}.provenance.json << 'PROVEOF'
           {
diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml
index a6d97d3..d8e108f 100644
--- a/.github/workflows/scorecard.yml
+++ b/.github/workflows/scorecard.yml
@@ -32,7 +32,7 @@ jobs:
           repo_token: ${{ secrets.SCORECARD_TOKEN || secrets.GITHUB_TOKEN }}
 
       - name: Upload Scorecard results to GitHub Security tab
-        uses: github/codeql-action/upload-sarif@v4 # tag required by scorecard webapp verification
+        uses: github/codeql-action/upload-sarif@256d634097be96e792d6764f9edaefc4320557b1 # v4
         with:
           sarif_file: results.sarif
           category: scorecard
diff --git a/nora-registry/src/ui/templates.rs b/nora-registry/src/ui/templates.rs
index 23ef658..9ceb182 100644
--- a/nora-registry/src/ui/templates.rs
+++ b/nora-registry/src/ui/templates.rs
@@ -75,43 +75,56 @@ pub fn render_dashboard(data: &DashboardResponse, lang: Lang) -> String {
         )
     } else {
         // Group consecutive identical entries (same action+artifact+registry+source)
-        let mut grouped: Vec<(String, String, String, String, String, usize)> = Vec::new();
+        struct GroupedActivity {
+            time: String,
+            action: String,
+            artifact: String,
+            registry: String,
+            source: String,
+            count: usize,
+        }
+
+        let mut grouped: Vec<GroupedActivity> = Vec::new();
         for entry in &data.activity {
             let action = entry.action.to_string();
-            let last_match = grouped
-                .last()
-                .map(|(_, a, art, reg, src, _)| {
-                    *a == action
-                        && *art == entry.artifact
-                        && *reg == entry.registry
-                        && *src == entry.source
-                })
-                .unwrap_or(false);
+            let is_repeat = grouped.last().is_some_and(|last| {
+                last.action == action
+                    && last.artifact == entry.artifact
+                    && last.registry == entry.registry
+                    && last.source == entry.source
+            });
 
-            if last_match {
-                grouped.last_mut().unwrap().5 += 1;
+            if is_repeat {
+                if let Some(last) = grouped.last_mut() {
+                    last.count += 1;
+                }
             } else {
-                let time_ago = format_relative_time(&entry.timestamp);
-                grouped.push((
-                    time_ago,
+                grouped.push(GroupedActivity {
+                    time: format_relative_time(&entry.timestamp),
                     action,
-                    entry.artifact.clone(),
-                    entry.registry.clone(),
-                    entry.source.clone(),
-                    1,
-                ));
+                    artifact: entry.artifact.clone(),
+                    registry: entry.registry.clone(),
+                    source: entry.source.clone(),
+                    count: 1,
+                });
             }
         }
 
         grouped
             .iter()
-            .map(|(time, action, artifact, registry, source, count)| {
-                let display_artifact = if *count > 1 {
-                    format!("{} (x{})", artifact, count)
+            .map(|g| {
+                let display_artifact = if g.count > 1 {
+                    format!("{} (x{})", g.artifact, g.count)
                 } else {
-                    artifact.clone()
+                    g.artifact.clone()
                 };
-                render_activity_row(time, action, &display_artifact, registry, source)
+                render_activity_row(
+                    &g.time,
+                    &g.action,
+                    &display_artifact,
+                    &g.registry,
+                    &g.source,
+                )
             })
             .collect()
     };