Static analysis fixes

app/assets/javascripts/foreman_remote_execution/template_invocation.js

@@ -47,7 +47,9 @@ function show_preview_hosts_modal() {
     url: $('#previewHostsModal').attr('data-url')
   }).then(function(result){
     var modal_window = $('#previewHostsModal');
-    modal_window.find('.modal-body').html(result);
+    // Sanitize the result to prevent XSS attacks
+    var sanitizedResult = $('<div>').text(result).html();
+    modal_window.find('.modal-body').html(sanitizedResult);
     modal_window.modal({'show': true});
     modal_window.find('a[rel="popover-modal"]').popover();
   });

app/controllers/cockpit_controller.rb

@@ -8,15 +8,23 @@ def host_ssh_params
   def redirect
     return invalid_request unless params[:redirect_uri]
 
-    redir_url = URI.parse(params[:redirect_uri])
+    begin
+      redir_url = URI.parse(params[:redirect_uri])
+    rescue URI::InvalidURIError
+      return invalid_request
+    end
+
+    # Validate URL scheme to prevent javascript: or data: schemes
+    return invalid_request unless %w[http https].include?(redir_url.scheme&.downcase)
 
     cockpit_url = ScriptExecutionProvider.cockpit_url_for_host('')
-    redir_url.query = if redir_url.hostname == URI.join(Setting[:foreman_url], cockpit_url).hostname
-                        "access_token=#{request.session_options[:id]}"
-                      else
-                        "error_description=Sorry"
-                      end
-    redirect_to(redir_url.to_s)
+    expected_hostname = URI.join(Setting[:foreman_url], cockpit_url).hostname
+
+    # Only redirect to expected hostname to prevent open redirects
+    return invalid_request unless redir_url.hostname == expected_hostname
+
+    redir_url.query = "access_token=#{request.session_options[:id]}"
+    redirect_to(redir_url.to_s, allow_other_host: true)
   end
 
   private

test/functional/cockpit_controller_test.rb

@@ -13,4 +13,69 @@ def setup
     response = ActiveSupport::JSON.decode(@response.body)
     assert response.key?('ssh_user'), 'ssh_params response must include ssh_user'
   end
+
+  describe 'redirect action security tests' do
+    setup do
+      Setting[:foreman_url] = 'https://foreman.example.com'
+      # Mock the cockpit URL
+      ScriptExecutionProvider.stubs(:cockpit_url_for_host).returns('/cockpit')
+    end
+
+    test "should reject redirect without redirect_uri parameter" do
+      get :redirect, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should reject redirect with invalid URI" do
+      get :redirect, params: { redirect_uri: 'not-a-valid-uri::' }, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should reject redirect with javascript scheme" do
+      get :redirect, params: { redirect_uri: 'javascript:alert("xss")' }, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should reject redirect with data scheme" do
+      get :redirect, params: { redirect_uri: 'data:text/html,<script>alert("xss")</script>' }, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should reject redirect with ftp scheme" do
+      get :redirect, params: { redirect_uri: 'ftp://evil.com/file' }, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should reject redirect to wrong hostname" do
+      get :redirect, params: { redirect_uri: 'https://evil.com/cockpit' }, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should reject redirect to subdomain attack" do
+      get :redirect, params: { redirect_uri: 'https://evil.foreman.example.com/cockpit' }, session: set_session_user
+      assert_response :bad_request
+    end
+
+    test "should allow redirect to valid cockpit URL" do
+      valid_uri = 'https://foreman.example.com/cockpit/path'
+      get :redirect, params: { redirect_uri: valid_uri }, session: set_session_user
+      assert_response :redirect
+
+      # Verify the redirect includes the access token
+      location = response.location
+      assert_includes location, 'access_token='
+      assert_includes location, 'https://foreman.example.com/cockpit/path'
+    end
+
+    test "should allow redirect with http scheme to same hostname" do
+      valid_uri = 'http://foreman.example.com/cockpit/path'
+      get :redirect, params: { redirect_uri: valid_uri }, session: set_session_user
+      assert_response :redirect
+    end
+
+    test "should handle case insensitive scheme validation" do
+      get :redirect, params: { redirect_uri: 'HTTPS://foreman.example.com/cockpit' }, session: set_session_user
+      assert_response :redirect
+    end
+  end
 end

webpack/JobInvocationDetail/JobInvocationConstants.js

@@ -74,7 +74,9 @@ const Columns = () => {
         return { title: __('Failed'), status: 1 };
       case 'planned':
         return { title: __('Scheduled'), status: 2 };
-      case 'running' || 'pending':
+      case 'running':
+        return { title: __('Pending'), status: 3 };
+      case 'pending':
         return { title: __('Pending'), status: 3 };
       case 'cancelled':
         return { title: __('Cancelled'), status: 4 };

webpack/JobInvocationDetail/JobInvocationHostTable.js

@@ -258,7 +258,7 @@ const JobInvocationHostTable = ({
     setAPIOptions: filterApiCall,
   };
 
-  const results = apiResponse.results ?? [];
+  const results = apiResponse?.results ?? [];
 
   const selectionToolbar = (
     <ToolbarItem key="selectAll">

webpack/JobInvocationDetail/TemplateInvocation.js

@@ -212,10 +212,10 @@ export const TemplateInvocation = ({
             <Alert
               variant="danger"
               ouiaId="template-invocation-preview-alert"
-              title={preview.plain}
+              title={preview?.plain}
             />
           ) : (
-            <pre className="template-invocation-preview">{preview.plain}</pre>
+            <pre className="template-invocation-preview">{preview?.plain}</pre>
           )}
         </>
       )}

webpack/JobInvocationDetail/__tests__/TemplateInvocation.test.js

@@ -144,4 +144,76 @@ describe('TemplateInvocation', () => {
 
     expect(document.querySelectorAll('.pf-v5-c-skeleton')).toHaveLength(1);
   });
+
+  test('handles null preview object gracefully', async () => {
+    const mockResponseWithNullPreview = {
+      ...mockTemplateInvocationResponse,
+      preview: null,
+    };
+
+    selectors.selectTemplateInvocationStatus.mockImplementation(() => () =>
+      'RESOLVED'
+    );
+    selectors.selectTemplateInvocation.mockImplementation(() => () =>
+      mockResponseWithNullPreview
+    );
+
+    render(
+      <Provider store={store}>
+        <TemplateInvocation
+          hostID="1"
+          jobID="1"
+          isInTableView={false}
+          isExpanded
+          hostName="example-host"
+          hostProxy={{ name: 'example-proxy', href: '#' }}
+        />
+      </Provider>
+    );
+
+    // Enable showCommand to trigger preview rendering
+    act(() => {
+      fireEvent.click(screen.getByText('Command'));
+    });
+
+    // Should not crash and should not display any preview content
+    expect(screen.queryByText('template-invocation-preview')).not.toBeInTheDocument();
+  });
+
+  test('handles undefined preview.plain gracefully', async () => {
+    const mockResponseWithUndefinedPreview = {
+      ...mockTemplateInvocationResponse,
+      preview: {}, // preview object exists but plain is undefined
+    };
+
+    selectors.selectTemplateInvocationStatus.mockImplementation(() => () =>
+      'RESOLVED'
+    );
+    selectors.selectTemplateInvocation.mockImplementation(() => () =>
+      mockResponseWithUndefinedPreview
+    );
+
+    render(
+      <Provider store={store}>
+        <TemplateInvocation
+          hostID="1"
+          jobID="1"
+          isInTableView={false}
+          isExpanded
+          hostName="example-host"
+          hostProxy={{ name: 'example-proxy', href: '#' }}
+        />
+      </Provider>
+    );
+
+    // Enable showCommand to trigger preview rendering
+    act(() => {
+      fireEvent.click(screen.getByText('Command'));
+    });
+
+    // Should not crash - the optional chaining should handle undefined preview.plain
+    expect(() => {
+      screen.getByText('template-invocation-preview');
+    }).not.toThrow();
+  });
 });

webpack/JobWizard/JobWizard.js

@@ -99,17 +99,21 @@ export const JobWizard = ({ rerunData }) => {
       if (inputs) {
         setTemplateValues(prev => {
           inputs.forEach(input => {
-            defaultTemplateValues[input.name] =
-              prev[input.name] || input?.default || '';
+            if (input) {
+              defaultTemplateValues[input.name] =
+                prev[input.name] || input.default || '';
+            }
           });
           return defaultTemplateValues;
         });
       }
       setAdvancedValues(currentAdvancedValues => {
         if (advancedInputs) {
           advancedInputs.forEach(input => {
-            advancedTemplateValues[input.name] =
-              currentAdvancedValues[input.name] || input?.default || '';
+            if (input) {
+              advancedTemplateValues[input.name] =
+                currentAdvancedValues[input.name] || input.default || '';
+            }
           });
         }
         return {
@@ -206,7 +210,7 @@ export const JobWizard = ({ rerunData }) => {
   const [isStartsAtError, setIsStartsAtError] = useState(false);
   useEffect(() => {
     const updateStartsError = () => {
-      if (scheduleValue.scheduleType === SCHEDULE_TYPES.FUTURE) {
+      if (scheduleValue?.scheduleType === SCHEDULE_TYPES.FUTURE) {
         setIsStartsAtError(
           !!scheduleValue?.startsAt?.length &&
             new Date().getTime() >= new Date(scheduleValue.startsAt).getTime()
@@ -216,7 +220,7 @@ export const JobWizard = ({ rerunData }) => {
             new Date().getTime() >=
               new Date(scheduleValue.startsBefore).getTime()
         );
-      } else if (scheduleValue.scheduleType === SCHEDULE_TYPES.RECURRING) {
+      } else if (scheduleValue?.scheduleType === SCHEDULE_TYPES.RECURRING) {
         setIsStartsAtError(
           !!scheduleValue?.startsAt?.length &&
             new Date().getTime() >= new Date(scheduleValue.startsAt).getTime()

webpack/JobWizard/JobWizardPageRerun.js

@@ -173,10 +173,12 @@ const JobWizardPageRerun = ({
             <Divider component="div" />
             <JobWizard
               rerunData={
-                {
-                  ...jobInvocationResponse?.job,
-                  inputs: jobInvocationResponse?.inputs,
-                } || null
+                jobInvocationResponse?.job
+                  ? {
+                      ...jobInvocationResponse.job,
+                      inputs: jobInvocationResponse?.inputs,
+                    }
+                  : null
               }
             />
           </React.Fragment>