Merge branch 'master' into fdf-133550-forms-service-separation

Author: wayne-weibel

.github/workflows/code_checks.yml

@@ -59,8 +59,114 @@ jobs:
           bundler-cache: true
 
       - name: Run bundle-audit (checks gems for CVE issues)
+        id: bundle_audit
+        continue-on-error: true
         run: bundle exec bundle-audit check --update
 
+      - name: Configure AWS Credentials
+        if: steps.bundle_audit.outcome == 'failure'
+        uses: aws-actions/configure-aws-credentials@v6
+        with:
+          role-to-assume: ${{ vars.AWS_ASSUME_ROLE }}
+          aws-region: us-gov-west-1
+
+      - uses: department-of-veterans-affairs/action-inject-ssm-secrets@d8e6de3bde4dd728c9d732baef58b3c854b8c4bb
+        if: steps.bundle_audit.outcome == 'failure'
+        with:
+          ssm_parameter: /devops/VA_VSP_BOT_GITHUB_TOKEN
+          env_variable_name: VA_VSP_BOT_GITHUB_TOKEN
+
+      - name: Checkout VSP actions
+        if: steps.bundle_audit.outcome == 'failure'
+        uses: actions/checkout@v6
+        with:
+          repository: department-of-veterans-affairs/vsp-github-actions
+          ref: refs/heads/main
+          token: ${{ env.VA_VSP_BOT_GITHUB_TOKEN }}
+          persist-credentials: false
+          path: ./.github/actions/vsp-github-actions
+
+      - uses: department-of-veterans-affairs/action-inject-ssm-secrets@d8e6de3bde4dd728c9d732baef58b3c854b8c4bb
+        if: steps.bundle_audit.outcome == 'failure'
+        with:
+          ssm_parameter: /devops/github_actions_slack_socket_token
+          env_variable_name: SLACK_APP_TOKEN
+
+      - uses: department-of-veterans-affairs/action-inject-ssm-secrets@d8e6de3bde4dd728c9d732baef58b3c854b8c4bb
+        if: steps.bundle_audit.outcome == 'failure'
+        with:
+          ssm_parameter: /devops/github_actions_slack_bot_user_token
+          env_variable_name: SLACK_BOT_TOKEN
+
+      - name: Slack notify - bundle-audit CVE failure
+        if: steps.bundle_audit.outcome == 'failure'
+        uses: ./.github/actions/vsp-github-actions/slack-socket
+        with:
+          slack_app_token: ${{ env.SLACK_APP_TOKEN }}
+          slack_bot_token: ${{ env.SLACK_BOT_TOKEN }}
+          message: "Critical: Vets-API bundle-audit detected CVE vulnerabilities"
+          blocks: |
+            [
+              { "type": "divider" },
+              {
+                "type": "header",
+                "text": {
+                  "type": "plain_text",
+                  "text": ":rotating_light: CRITICAL: CVE Vulnerability Detected",
+                  "emoji": true
+                }
+              },
+              {
+                "type": "section",
+                "text": {
+                  "type": "mrkdwn",
+                  "text": "*bundle-audit found gems with known CVE vulnerabilities in vets-api*"
+                }
+              },
+              {
+                "type": "section",
+                "fields": [
+                  {
+                    "type": "mrkdwn",
+                    "text": ":x: *Status*: Build Failed"
+                  },
+                  {
+                    "type": "mrkdwn",
+                    "text": ":git: *Branch*: `${{ github.ref_name }}`"
+                  }
+                ]
+              },
+              {
+                "type": "section",
+                "text": {
+                  "type": "mrkdwn",
+                  "text": ":bust_in_silhouette: *Author:* ${{ github.actor }}"
+                }
+              },
+              {
+                "type": "actions",
+                "elements": [
+                  {
+                    "type": "button",
+                    "text": {
+                      "type": "plain_text",
+                      "text": ":mag_right: View Run",
+                      "emoji": true
+                    },
+                    "url": "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}",
+                    "style": "danger"
+                  }
+                ]
+              },
+              { "type": "divider" }
+            ]
+          channel_id: "C0APA867M0V"
+
+      - name: Fail job if bundle-audit found CVEs
+        if: steps.bundle_audit.outcome == 'failure'
+        run: exit 1
+
+
       - name: Run Rubocop
         run: bundle exec rubocop --parallel --format github
 
@@ -569,7 +675,7 @@ jobs:
               },
               { "type": "divider" }
             ]
-          channel_id: "C039HRTHXDH"
+          channel_id: "C0APA867M0V"
 
       - name: Slack notify - warning low coverage alert
         if: needs.tests.result == 'success' && github.event_name != 'pull_request' && fromJSON(steps.coverage.outputs.percent) >= 90 && fromJSON(steps.coverage.outputs.percent) < 93
@@ -639,7 +745,7 @@ jobs:
               },
               { "type": "divider" }
             ]
-          channel_id: "C039HRTHXDH"
+          channel_id: "C0APA867M0V"
 
       - name: Comment Coverage on PR
         if: needs.tests.result == 'success' && github.event_name == 'pull_request' && fromJSON(steps.coverage.outputs.percent) < 90

.github/workflows/deploy_delay_notifications.yml

@@ -151,7 +151,7 @@ jobs:
               },
               { "type": "divider" }
             ]
-          channel_id: "C039HRTHXDH"
+          channel_id: "C0APA867M0V"
 
       - name: Notify for other failure
         if: ${{ env.dev_summary == '' && env.staging_summary == '' }}
@@ -172,4 +172,4 @@ jobs:
               },
               { "type": "divider" }
             ]
-          channel_id: "C039HRTHXDH"
+          channel_id: "C0APA867M0V"

.github/workflows/deploy_missing_notifications.yml

@@ -175,4 +175,4 @@ jobs:
           slack_bot_token: ${{ env.SLACK_BOT_TOKEN }}
           message: "Vets API Deployment Delay:"
           blocks: "[{\"type\": \"divider\"}, {\"type\": \"section\", \"text\": { \"type\": \"mrkdwn\", \"text\": \":scared_and_sweating_smiley: GitHub Action Runner Workflow failed! :scared_and_sweating_smiley:\n <https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}|${{ github.workflow }} Run #${{ github.run_number }}>\n\n*Status Summary:*\n${{ needs.check-api-status.outputs.status_summary }}\"}}, {\"type\": \"divider\"}]"
-          channel_id: "C039HRTHXDH"
+          channel_id: "C0APA867M0V"

Gemfile.lock

@@ -242,7 +242,7 @@ GEM
       minitest (>= 5.1, < 6)
       securerandom (>= 0.3)
       tzinfo (~> 2.0, >= 2.0.5)
-    addressable (2.8.8)
+    addressable (2.8.9)
       public_suffix (>= 2.0.2, < 8.0)
     adler32 (0.0.2)
     afm (1.0.0)
@@ -441,7 +441,7 @@ GEM
       dry-logic (~> 1.6)
       dry-types (~> 1.8)
       zeitwerk (~> 2.6)
-    dry-struct (1.8.0)
+    dry-struct (1.8.1)
       dry-core (~> 1.1)
       dry-types (~> 1.8, >= 1.8.2)
       ice_nine (~> 0.11)
@@ -473,7 +473,7 @@ GEM
     factory_bot_rails (6.5.1)
       factory_bot (~> 6.5)
       railties (>= 6.1.0)
-    faker (3.6.0)
+    faker (3.6.1)
       i18n (>= 1.8.11, < 2)
     faraday (2.14.1)
       faraday-net_http (>= 2.0, < 3.5)
@@ -497,12 +497,12 @@ GEM
     faraday_curl (0.0.2)
       faraday (>= 0.9.0)
     fastimage (2.4.1)
-    ffi (1.17.3)
-    ffi (1.17.3-aarch64-linux-gnu)
-    ffi (1.17.3-java)
-    ffi (1.17.3-x64-mingw-ucrt)
-    ffi (1.17.3-x86-mingw32)
-    ffi (1.17.3-x86_64-linux-gnu)
+    ffi (1.17.4)
+    ffi (1.17.4-aarch64-linux-gnu)
+    ffi (1.17.4-java)
+    ffi (1.17.4-x64-mingw-ucrt)
+    ffi (1.17.4-x86-mingw32)
+    ffi (1.17.4-x86_64-linux-gnu)
     ffi-compiler (1.3.2)
       ffi (>= 1.15.5)
       rake
@@ -583,26 +583,26 @@ GEM
       base64 (~> 0.2)
       faraday (>= 1.0, < 3.a)
     google-logging-utils (0.2.0)
-    google-protobuf (4.33.5)
+    google-protobuf (4.34.0)
       bigdecimal
-      rake (>= 13)
-    google-protobuf (4.33.5-aarch64-linux-gnu)
+      rake (~> 13.3)
+    google-protobuf (4.34.0-aarch64-linux-gnu)
       bigdecimal
-      rake (>= 13)
-    google-protobuf (4.33.5-java)
+      rake (~> 13.3)
+    google-protobuf (4.34.0-java)
       bigdecimal
       ffi (~> 1)
       ffi-compiler (~> 1)
-      rake (>= 13)
-    google-protobuf (4.33.5-x64-mingw-ucrt)
+      rake (~> 13.3)
+    google-protobuf (4.34.0-x64-mingw-ucrt)
       bigdecimal
-      rake (>= 13)
-    google-protobuf (4.33.5-x86-mingw32)
+      rake (~> 13.3)
+    google-protobuf (4.34.0-x86-mingw32)
       bigdecimal
-      rake (>= 13)
-    google-protobuf (4.33.5-x86_64-linux-gnu)
+      rake (~> 13.3)
+    google-protobuf (4.34.0-x86_64-linux-gnu)
       bigdecimal
-      rake (>= 13)
+      rake (~> 13.3)
     googleauth (1.16.2)
       faraday (>= 1.0, < 3.a)
       google-cloud-env (~> 2.2)
@@ -1248,7 +1248,7 @@ GEM
       activemodel (>= 6.0.0)
       bindex (>= 0.4.0)
       railties (>= 6.0.0)
-    webmock (3.26.1)
+    webmock (3.26.2)
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)

app/sidekiq/simple_forms_api/form_remediation/upload_retry_job.rb

@@ -8,8 +8,10 @@ module FormRemediation
     # +Aws::S3::Errors::ServiceError+ occurs during the initial upload attempt.
     #
     # @example Enqueue a retry (typically called from Uploader, not directly)
-    #   UploadRetryJob.perform_async(file, directory, config)
-    #
+    # UploadRetryJob.perform_async(temp_file_path, directory, config_class_name)
+    # we now pass the temp file path instead of the original file object because the retry job needs to be able
+    # access the file even if the original file object
+    # has been cleaned up by the caller after the initial upload attempt
     # @see SimpleFormsApi::FormRemediation::Uploader
     class UploadRetryJob
       include Sidekiq::Job
@@ -22,11 +24,20 @@ class FileNoLongerExistsError < StandardError; end
 
       STATSD_KEY_PREFIX = 'api.simple_forms_api.upload_retry_job'
 
-      sidekiq_retries_exhausted do |_msg, ex|
+      sidekiq_retries_exhausted do |msg, ex|
+        temp_path  = msg['args']&.first.to_s
+        s3_dir     = msg['args']&.[](1).to_s
         StatsD.increment("#{STATSD_KEY_PREFIX}.retries_exhausted")
+
         Rails.logger.error(
-          'SimpleFormsApi::FormRemediation::UploadRetryJob retries exhausted',
-          { exception: "#{ex.class} - #{ex.message}", backtrace: ex.backtrace&.join("\n").to_s }
+          'SimpleFormsApi::FormRemediation::UploadRetryJob retries exhausted - ' \
+          'manual remediation required. Temp file preserved for recovery.',
+          {
+            exception: "#{ex.class} - #{ex.message}",
+            backtrace: ex.backtrace&.first(10)&.join("\n").to_s,
+            temp_path:,
+            s3_directory: s3_dir
+          }
         )
       end
 
@@ -40,9 +51,10 @@ class FileNoLongerExistsError < StandardError; end
       # (where arguments are JSON-serialized into strings/hashes). Each argument is
       # normalized to the expected type before use.
       #
-      # @param file [String, Hash, CarrierWave::SanitizedFile] file path, serialized hash, or file object
+      # @param file [String, Hash, CarrierWave::SanitizedFile] temp file path,
+      #   serialised hash, or file object
       # @param directory [String] the S3 target directory
-      # @param config [String, Configuration::Base] config class name or config instance
+      # @param config [String, Configuration::Base] config class name or instance
       def perform(file, directory, config)
         @file = file.respond_to?(:filename) ? file : CarrierWave::SanitizedFile.new(file)
         @directory = directory
@@ -52,31 +64,38 @@ def perform(file, directory, config)
 
         uploader = @config.uploader_class.new(directory:, config: @config)
 
-        begin
-          StatsD.increment("#{STATSD_KEY_PREFIX}.total")
+        StatsD.increment("#{STATSD_KEY_PREFIX}.total")
 
-          uploader.store!(@file)
+        begin
+          uploader.store_for_retry!(@file)
         rescue Aws::S3::Errors::ServiceError
           raise if service_available?(@config.s3_settings.region)
 
           retry_later
+          return
         end
+
+        # Upload succeeded — remove the temp copy inside this job.
+        cleanup_temp_file!
+
+        StatsD.increment("#{STATSD_KEY_PREFIX}.success")
       end
 
       private
 
       attr_accessor :file, :directory, :config
 
-      # Raises if the file no longer exists on disk. Callers typically delete
-      # the source file after the initial upload attempt, so retries may arrive
-      # with a stale path. Failing fast avoids burning Sidekiq retries.
+      # Raises {FileNoLongerExistsError} if the temp file is missing.
+      # This kills the Sidekiq job immediately rather than burning retries on
+      # a file that will never reappear.
       def verify_file_exists!(sanitized_file)
         path = sanitized_file.respond_to?(:path) ? sanitized_file.path : sanitized_file.to_s
         return if path.blank? || File.exist?(path)
 
         StatsD.increment("#{STATSD_KEY_PREFIX}.file_missing")
         raise FileNoLongerExistsError,
-              "Retry file no longer exists at #{path}. Source was likely cleaned up after the initial upload failure."
+              "Retry file no longer exists at #{path}. " \
+              'The temp copy may have been removed by a previous successful attempt or manual cleanup.'
       end
 
       # Checks whether the S3 service is reachable in the given region.
@@ -91,13 +110,38 @@ def service_available?(region)
       end
 
       # Re-enqueues this job with a delay when S3 is completely unavailable.
-      # This avoids burning a Sidekiq retry on a known-down service.
+      # Uses the current +file.path+ (which already points to the temp copy)
+      # so the same file is available when the delayed job runs.
       #
-      # @param delay [Time] when to run the retry (default: 30 minutes from now)
+      # @param delay [ActiveSupport::Duration] when to run the retry
       def retry_later(delay: 30.minutes.from_now)
-        Rails.logger.info("S3 service unavailable. Retrying upload later for #{file.filename}.")
+        Rails.logger.info(
+          'SimpleFormsApi::FormRemediation::UploadRetryJob - S3 service unavailable, scheduling retry',
+          { temp_path: file.path, s3_directory: directory, retry_at: delay }
+        )
         self.class.perform_in(delay, file.path, directory, config.class.name)
       end
+
+      # Deletes the temp copy that {Uploader#store!} created for this job.
+      # Logs a warning instead of raising if the file is already gone — a
+      # concurrent process may have cleaned it up, which is not an error.
+      def cleanup_temp_file!
+        path = file.respond_to?(:path) ? file.path : file.to_s
+        return if path.blank?
+
+        if File.exist?(path)
+          FileUtils.rm_f(path)
+          Rails.logger.info(
+            'SimpleFormsApi::FormRemediation::UploadRetryJob - temp file removed after successful upload',
+            { temp_path: path }
+          )
+        else
+          Rails.logger.warn(
+            'SimpleFormsApi::FormRemediation::UploadRetryJob - temp file already gone at cleanup; skipping',
+            { temp_path: path }
+          )
+        end
+      end
     end
   end
 end