self-hosted: refactored block-policy apply logic

[h0x0er]

This PR changes the way block-policy is applied in case of self-hosted runners.

New logic supports block-policy enforcement in containerized workflows (self-hosted-scenario)

[step-security-bot]

Please find StepSecurity AI-CodeWise code comments below.

Code Comments

src/cleanup.ts

[
    {
        "Severity": "High",
        "Recommendation": "Use proper naming conventions for functions and variables.",
        "Description": "Inconsistent naming conventions can lead to confusion and errors.",
        "Remediation": "Change 'isArcRunner' function to 'isARCRunner' for consistency."
    },
    {
        "Severity": "Medium",
        "Recommendation": "Avoid using synchronous execSync calls for shell commands.",
        "Description": "Synchronous calls can block the event loop and impact application performance.",
        "Remediation": "Refactor the code to use asynchronous alternatives like execFile and handle the callbacks properly."
    },
    {
        "Severity": "Low",
        "Recommendation": "Use consistent code formatting for object literals.",
        "Description": "Inconsistent formatting can make the code harder to read and maintain.",
        "Remediation": "Ensure consistent indentation and formatting of object literal properties."
    }
]

dist/index.js.map

[]

dist/post/index.js.map

[]

dist/pre/index.js.map

[]

src/arc-runner.ts

[
    {
        "Severity": "High",
        "Recommendation": "Update function name to match naming conventions",
        "Description": "The function name isARCRunner() should follow camel case naming conventions.",
        "Remediation": "Change the function name from isARCRunner to isArcRunner."
    },
    {
        "Severity": "High",
        "Recommendation": "Avoid using console.log for logging sensitive information",
        "Description": "Logging sensitive information like sent endpoints is a security risk.",
        "Remediation": "Replace console.log with a more secure logging mechanism, such as a dedicated logging library or service."
    },
    {
        "Severity": "Medium",
        "Recommendation": "Avoid using environment variables for security-sensitive operations",
        "Description": "Using process.env directly for security-sensitive checks can expose the application to vulnerabilities.",
        "Remediation": "Consider using a more secure storage mechanism, such as a secrets manager, for storing and accessing sensitive information."
    },
    {
        "Severity": "Low",
        "Recommendation": "Use strict comparison (===) for checking undefined",
        "Description": "Using loose comparison (!==) for checking if process.env['KUBERNETES_PORT'] is undefined may lead to unexpected behavior.",
        "Remediation": "Change the comparison from process.env['KUBERNETES_PORT'] !== undefined to process.env['KUBERNETES_PORT'] !== undefined."
    }
]

src/common.ts

[
    {
        "Severity": "Low",
        "Recommendation": "Use clear and descriptive variable names to improve code readability and maintainability.",
        "Description": "The variable `SELF_HOSTED_RUNNER_MESSAGE` might need a more descriptive name to convey its purpose clearly.",
        "Remediation": "export const SELF_HOSTED_RUNNER_NOTIFICATION = 'This job is running on a self-hosted runner.';"
    },
    {
        "Severity": "Low",
        "Recommendation": "Consistent error handling and messaging.",
        "Description": "The messaging in the `SELF_HOSTED_RUNNER_MESSAGE` variable does not make it clear that there is a negative impact by being on a self-hosted runner without Harden-Runner installed.",
        "Remediation": "export const SELF_HOSTED_RUNNER_MESSAGE = 'This job is running on a self-hosted runner without Harden-Runner installed. This job will not be monitored.';"
    }
]

src/setup.ts

[
    {
        "Severity": "High",
        "Recommendation": "Use consistent naming conventions for functions, variables, and modules.",
        "Description": "Inconsistent naming conventions can lead to confusion and make code harder to understand and maintain.",
        "Remediation": "Change 'isArcRunner' to 'isARCRunner' in the import statement and function definition."
    },
    {
        "Severity": "High",
        "Recommendation": "Use consistent formatting for function calls with multiple arguments.",
        "Description": "Inconsistent formatting can make the code less readable and harder to maintain.",
        "Remediation": "Update the call to 'core.getBooleanInput' to have each argument on a new line for better readability."
    },
    {
        "Severity": "Medium",
        "Recommendation": "Avoid using undefined variables directly without checking if they exist.",
        "Description": "Accessing properties of undefined variables can lead to runtime errors.",
        "Remediation": "Add a null check for 'context.payload.repository.private' before accessing its properties in the 'MonitorResponse' interface."
    },
    {
        "Severity": "Medium",
        "Recommendation": "Remove unreachable code to improve code clarity.",
        "Description": "Unreachable code can confuse developers and should be removed to keep the codebase clean.",
        "Remediation": "Remove the unreachable code block after the 'core.info(common.SELF_HOSTED_RUNNER_MESSAGE);' statement."
    },
    {
        "Severity": "Medium",
        "Recommendation": "Consolidate common operations to avoid code duplication and improve maintainability.",
        "Description": "Repeated blocks of code can be consolidated into functions to improve code readability and maintainability.",
        "Remediation": "Move the common logic shared between both if conditions into a separate function for better code organization."
    },
    {
        "Severity": "Low",
        "Recommendation": "Use async/await consistently for handling asynchronous code.",
        "Description": "Consistent use of async/await syntax improves code readability and maintainability.",
        "Remediation": "Ensure that asynchronous operations within the 'if (confg.egress_policy === 'block')' block also use async/await syntax."
    }
]

dist/index.js

[
    {
        "Severity": "High",
        "Recommendation": "Avoid revealing sensitive information in error messages",
        "Description": "The SELF_HOSTED_RUNNER_MESSAGE could potentially disclose information about the infrastructure to attackers.",
        "Remediation": "Modify the message to be more generic and not reveal details about the runner setup."
    },
    {
        "Severity": "Medium",
        "Recommendation": "Follow a consistent naming convention for messages",
        "Description": "The naming convention for message constants should be uniform. Inconsistent naming can lead to confusion.",
        "Remediation": "Update the message constant to follow the same naming convention as the other messages."
    }
]

dist/post/index.js

[
    {
        "Severity": "High",
        "Recommendation": "Update SELF_HOSTED_RUNNER_MESSAGE to be more informative and relevant to the context.",
        "Description": "The message 'This job is running on a self-hosted runner.' could be improved to provide more useful information.",
        "Remediation": "const SELF_HOSTED_RUNNER_MESSAGE = 'This job is running on a self-hosted runner, but Harden-Runner is not installed. Harden-Runner is required for monitoring this job.';"
    },
    {
        "Severity": "High",
        "Recommendation": "Change function name isARCRunner to isARCRunner to follow camelCase naming convention.",
        "Description": "The function name 'isArcRunner' should be updated to 'isARCRunner' for consistency and readability.",
        "Remediation": "function isARCRunner() {"
    },
    {
        "Severity": "High",
        "Recommendation": "Correct the variable naming in the for loop in sendAllowedEndpoints function.",
        "Description": "The loop variable 'endpoint' should be declared using 'let' inside the loop to limit its scope.",
        "Remediation": "for (let endpoint of allowedEndpoints) {"
    },
    {
        "Severity": "Medium",
        "Recommendation": "Ensure trimming of endpoint in sendAllowedEndpoints function before processing.",
        "Description": "Adding a call to trim() on the endpoint variable ensures any leading or trailing whitespaces are removed before processing.",
        "Remediation": "endpoint = endpoint.trim();"
    },
    {
        "Severity": "Medium",
        "Recommendation": "Add a check to verify the length of trimmed endpoint before processing in sendAllowedEndpoints function.",
        "Description": "Adding a check for the length of the trimmed endpoint ensures valid endpoints are processed.",
        "Remediation": "if (endpoint.length > 0) {"
    },
    {
        "Severity": "Medium",
        "Recommendation": "Update the condition in sendAllowedEndpoints function to check 'sent' variable instead of allowedEndpoints length.",
        "Description": "The condition should be based on the 'sent' counter to accurately determine if any endpoints were sent.",
        "Remediation": "if (sent > 0) {"
    },
    {
        "Severity": "Low",
        "Recommendation": "Optimize logging with timestamp in sendAllowedEndpoints function for better performance.",
        "Description": "Adding a start time and logging the duration of the function can help in performance monitoring.",
        "Remediation": "const startTime = Date.now();\nconsole.log('[harden-runner] sendAllowedEndpoints completed in ${duration}ms (sent ${sent} endpoints)');"
    },
    {
        "Severity": "Low",
        "Recommendation": "Add a check for the presence of 'KUBERNETES_PORT' in isSecondaryPod function.",
        "Description": "It is recommended to explicitly check for the presence of 'KUBERNETES_PORT' variable before using it.",
        "Remediation": "let hasKubeEnv = process.env['KUBERNETES_PORT'] !== undefined;"
    }
]

dist/pre/index.js

[
    {
        "Severity": "High",
        "Recommendation": "Use descriptive variable names for improved readability",
        "Description": "Variable names like 'sent' and 'endpointPolicyStr' should be more descriptive to improve code readability and maintainability",
        "Remediation": "Rename variable 'sent' to 'sentEndpoints' for clarity; Rename variable 'endpointPolicyStr' to 'encodedEndpointPolicy' for clarity"
    },
    {
        "Severity": "High",
        "Recommendation": "Use consistent casing for function names",
        "Description": "Function names should adhere to a consistent casing convention for better code consistency",
        "Remediation": "Change function name 'isArcRunner' to 'isARCRunner' to follow consistent camelCase naming convention"
    },
    {
        "Severity": "Medium",
        "Recommendation": "Ensure error handling for file system operations",
        "Description": "File system operations like reading and writing files should have proper error handling to handle potential failures",
        "Remediation": "Implement try-catch blocks for file operations to handle and log potential errors"
    },
    {
        "Severity": "Medium",
        "Recommendation": "Avoid magic numbers in code",
        "Description": "Avoid using magic numbers directly in the code; instead, assign them to named constants for better code readability and maintainability",
        "Remediation": "Define a constant for '5000' and use it in place of the magic number in the code"
    },
    {
        "Severity": "Low",
        "Recommendation": "Use template literals for string interpolation",
        "Description": "Use ES6 template literals for string interpolation instead of traditional string concatenation for cleaner and more concise code",
        "Remediation": "Replace string concatenations with template literals where applicable for better readability"
    }
]

Feedback

We appreciate your feedback in helping us improve the service! To provide feedback, please use emojis on this comment. If you find the comments helpful, give them a 👍. If they aren't useful, kindly express that with a 👎. If you have questions or detailed feedback, please create n GitHub issue in StepSecurity/AI-CodeWise.

[varunsh-coder]

@h0x0er tests are failing. also can you please create PR into rc-21 branch?

[step-security-bot]

Code Review Skipped

StepSecurity AI-CodeWise is designed to handle a maximum of 10 file changes per pull request. To utilize its capabilities, please create a new pull request containing no more than 10 files

Feedback

We appreciate your feedback in helping us improve the service! To provide feedback, please use emojis on this comment. If you find the comments helpful, give them a 👍. If they aren't useful, kindly express that with a 👎. If you have questions or detailed feedback, please create n GitHub issue in StepSecurity/AI-CodeWise.

[step-security-bot]

Code Review Skipped

StepSecurity AI-CodeWise is designed to handle a maximum of 10 file changes per pull request. To utilize its capabilities, please create a new pull request containing no more than 10 files

Feedback

We appreciate your feedback in helping us improve the service! To provide feedback, please use emojis on this comment. If you find the comments helpful, give them a 👍. If they aren't useful, kindly express that with a 👎. If you have questions or detailed feedback, please create n GitHub issue in StepSecurity/AI-CodeWise.