Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate Website #46

Merged
merged 28 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@

jobs:

build:

Check warning on line 8 in .github/workflows/ci-build.yml

View workflow job for this annotation

GitHub Actions / 🔨 Build & Verify / Build

MissingJobTimeout

Job[build] is missing `timeout-minutes`. See also the [online documentation](https://ghlint.twisterrob.net/issues/default/MissingJobTimeout/.
name: "Build"
runs-on: ubuntu-latest
timeout-minutes: 5

permissions:
# read: actions/checkout, write: gradle/actions/setup-gradle
Expand Down
84 changes: 84 additions & 0 deletions .github/workflows/website.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
name: "Website"
on:
push:

jobs:
generate:
name: "Generate website"
runs-on: ubuntu-latest
timeout-minutes: 5

permissions:
# actions/checkout
contents: read

defaults:
run:
shell: bash

steps:
- name: "Checkout ${{ github.ref }} branch in ${{ github.repository }} repository."
uses: actions/checkout@v4

- name: "Set up Java."
uses: actions/setup-java@v4
with:
java-version-file: .java-version
distribution: temurin

- name: "Set up Gradle."
uses: gradle/actions/setup-gradle@v3
with:
cache-encryption-key: ${{ secrets.GRADLE_ENCRYPTION_KEY }}

- name: "Build & Verify project."
run: >
./gradlew
:ghlint-docs:run

- name: "Set up Python"
uses: actions/setup-python@v5
with:
python-version-file: 'website/.python-version'

- name: "Install dependencies"
working-directory: website
run: |
pip install -r requirements.txt

- name: "Retrieve GitHub Pages configuration."
id: pages
uses: actions/configure-pages@v4

- name: "Build the website"
env:
VERBOSE: ${{ env.ACTIONS_STEP_DEBUG && ' --verbose' || '' }}
working-directory: website
run: |
mkdocs build${VERBOSE} --clean --strict

- name: "Upload 'github-pages' artifact."
uses: actions/upload-pages-artifact@v3
with:
path: website/site/


deploy:

Check warning on line 66 in .github/workflows/website.yml

View workflow job for this annotation

GitHub Actions / 🔨 Build & Verify / Build

DoubleCurlyIf

Job[deploy] does not have double-curly-braces. See also the [online documentation](https://ghlint.twisterrob.net/issues/default/DoubleCurlyIf/.

Check warning on line 66 in .github/workflows/website.yml

View workflow job for this annotation

GitHub Actions / 🔨 Build & Verify / Build

MissingJobTimeout

Job[deploy] is missing `timeout-minutes`. See also the [online documentation](https://ghlint.twisterrob.net/issues/default/MissingJobTimeout/.
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed
name: "Deploy GitHub Pages Site"
if: github.event_name == 'push'
needs:
- generate

permissions:
pages: write
id-token: write

environment:
name: github-pages
url: ${{ steps.deployment.outputs.page_url }}

runs-on: ubuntu-latest
steps:
- name: "Deploy to GitHub Pages"
id: deployment
uses: actions/deploy-pages@v4
21 changes: 21 additions & 0 deletions modules/ghlint-docs/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
plugins {
id("net.twisterrob.ghlint.cli")
}

dependencies {
implementation(projects.ghlintRules)
}

application {
mainClass = "net.twisterrob.ghlint.docs.MainKt"
}

tasks.named("run", JavaExec.class) {
def output = rootProject.layout.projectDirectory.dir("website/docs/issues")
outputs.dir(output)
args(output)
}

tasks.named("build") {
dependsOn("run")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package net.twisterrob.ghlint.docs

import net.twisterrob.ghlint.rule.Issue
import net.twisterrob.ghlint.rule.Rule
import net.twisterrob.ghlint.rule.descriptionWithExamples
import net.twisterrob.ghlint.ruleset.RuleSet
import java.nio.file.Path
import kotlin.io.path.createDirectories
import kotlin.io.path.writeText

internal class Generator(
private val target: Path,
) {

fun generate(vararg ruleSets: RuleSet) {
ruleSets.forEach { ruleSet ->
val ruleSetFolder = target.resolve(ruleSet.id).apply { createDirectories() }
val ruleSetFile = ruleSetFolder.resolve("index.md")
ruleSetFile.writeText(
generateDocs(
ruleSet = ruleSet
)
)
ruleSet.createRules().forEach { rule ->
rule.issues.forEach { issue ->
val issueFile = ruleSetFolder.resolve(issue.fileName)
issueFile.writeText(
generateDocs(
ruleSet = ruleSet,
rule = rule,
issue = issue,
relatedIssues = rule.issues - issue
)
)
}
}
}
}

private fun generateDocs(ruleSet: RuleSet): String =
buildString {
appendLine(
"""
# Rule set "${ruleSet.name}" (`${ruleSet.id}`)

""".trimIndent()
)
ruleSet.createRules().sortedBy { it.id }.forEach { rule ->
appendLine(" - `${rule.id}`")
rule.issues.sortedBy { it.id }.forEach { issue ->
appendLine(" - [`${issue.id}`](${issue.fileName}): ${issue.title}")
}
}
}

private fun generateDocs(ruleSet: RuleSet, rule: Rule, issue: Issue, relatedIssues: List<Issue>): String =
buildString {
val relatedIssuesText = relatedIssues.joinToString(separator = ", ") { "[`${it.id}`](${it.fileName})" }
val related = if (relatedIssuesText.isNotEmpty()) " along with ${relatedIssuesText}" else ""
appendLine(
"""
# `${issue.id}`
${issue.title}

_Defined by `${rule.id}` in the "[${ruleSet.name}](../)" ruleset${related}._

## Description
""".trimIndent()
)
appendLine(issue.descriptionWithExamples)
}
}

private val Rule.id: String
get() = this::class.simpleName ?: error("Invalid rule")

private val Issue.fileName: String
get() = this.id + ".md"
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package net.twisterrob.ghlint.docs

import net.twisterrob.ghlint.rules.DefaultRuleSet
import java.nio.file.Path

public fun main(vararg args: String) {
val target = Path.of(args[0])
Generator(target).generate(DefaultRuleSet())
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ private fun reportingDescriptor(issue: Issue): ReportingDescriptor =
),
help = MultiformatMessageString(
text = "See help markdown.",
markdown = issue.descriptionWithExamples,
markdown = issue.descriptionWithExamples + issue.helpLink,
),
helpURI = "https://ghlint.twisterrob.net/issues/default/${issue.id}/", // not visible on GH UI.
// TODO defaultConfiguration = ReportingConfiguration(level = issue.severity), //
// TODO helpURI = "https://example.com/help", // not visible on GH UI.
// TODO properties = PropertyBag(tags = listOf("tag1", "tag2")), // visible in detail view on GH UI.
)

Expand Down Expand Up @@ -125,3 +125,6 @@ private fun result(finding: Finding, base: Path): Result {
),
)
}

private val Issue.helpLink: String
get() = "\n\nSee also the [online documentation](https://ghlint.twisterrob.net/issues/default/${id}/."
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public class DoubleCurlyIfRule : VisitorRule {

However, this condition will **always** evaluate to `true`:
The way to interpret this expression is as follows:

* Evaluate `steps.calculation.outputs.result` -> `'hello'`
* Substitute `'hello'` -> `if: hello == 'world'`
* Evaluate `"hello == 'world'"` -> `true`
Expand All @@ -152,7 +153,7 @@ public class DoubleCurlyIfRule : VisitorRule {
That string is then passed to `if`, but it's a non-empty string, which is truthy.

To confirm this, you can run a workflow with step debugging turned on, and you'll see this:
```
```log
Evaluating: (success() && format('{0} == ''world''', steps.calculation.outputs.result))
```
""".trimIndent(),
Expand All @@ -174,8 +175,10 @@ public class DoubleCurlyIfRule : VisitorRule {
Looking at the expression, it might be interpreted (by humans) as a valid boolean expression,
because the GitHub Actions Context variable accesses are wrapped in an Expression as expected.

However, this condition will **always** evaluate to `true`:
However, this condition will **always** evaluate to `true`.

The way to interpret this expression is as follows:

* Evaluate first `${{ }}` -> for example `true`
* Evaluate second `${{ }}` -> for example `false`
* Substitute expressions -> `if: 'true && false'`
Expand All @@ -186,7 +189,7 @@ public class DoubleCurlyIfRule : VisitorRule {
That string is then passed to `if`, but it's a non-empty string, which is truthy.

To confirm this, you can run a workflow with step debugging turned on, and you'll see this:
```
```log
Evaluating: (success() && format('{0} && {1}', ..., ...))
```
""".trimIndent(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class DuplicateShellRule : VisitorRule {
Step shells should be uniform for each job, since each step is executed on the same runner.

Relevant documentation:

* [`jobs.<job_id>.steps[*].shell`](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell)
* [`jobs.<job_id>.defaults.run.shell`](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_iddefaultsrun)
""".trimIndent(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class EnvironmentFileOverwriteRule : VisitorRule {
Use `>>` instead of `>`, to ensure the file is appended, not overwritten.

References:

* [Environment files documentation](https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#environment-files)
""".trimIndent(),
compliant = listOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ public class ExplicitJobPermissionsRule : VisitorRule {
Declaring permissions for the `github.token` / `secrets.GITHUB_TOKEN` temporary Access Token is the best practice.

There are three ways to declare permissions:

* on the repository level
* on the workflow level
* on the job level

The recommended setting is:

* Set the organization/repository level permissions to ["Read repository contents and packages permissions"](https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/#setting-the-default-permissions-for-the-organization-or-repository).
Sadly, the default is "Read and write permissions" (for everything), which is too permissive.
* Do not declare anything on the workflow level.
Expand All @@ -60,6 +62,7 @@ public class ExplicitJobPermissionsRule : VisitorRule {
```

References:

* [Documentation of `GITHUB_TOKEN` permissions](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token)
* [What can go wrong if a too permissive token leaks?](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#potential-impact-of-a-compromised-runner)
* [List of Available permissions](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token)
Expand Down Expand Up @@ -118,6 +121,7 @@ public class ExplicitJobPermissionsRule : VisitorRule {
Move the permissions declaration from the workflow level to the job level.

References:

* [Best practice in documentation](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token:~:text=The%20two,permissions%27%20scope.)
""".trimIndent(),
compliant = listOf(
Expand All @@ -141,12 +145,14 @@ public class ExplicitJobPermissionsRule : VisitorRule {
Permissions are declared on the workflow level, leading to escalated privileges for all jobs.

There are two jobs: `build` and `comment`:

* The `build` job needs to access contents to invoke the `make` command.
* The `comment` job needs to write comments to the pull request.

With the `permissions:` being declared on the workflow,
both jobs will have the same permissions.
This leads to a larger attack surface:

* The comment job will be able to read the repository contents.
This means that if the publish-comment-action is compromised,
it can read/steal the repository contents.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class FailFastActionsRule : VisitorRule {

val FailFastUploadArtifact = Issue(
id = "FailFastUploadArtifact",
title = "upload-artifact should fail fast.",
title = "`upload-artifact` should fail fast.",
description = """
`actions/upload-artifact` should be configured to fail the CI when no files are found.

Expand Down Expand Up @@ -101,7 +101,7 @@ public class FailFastActionsRule : VisitorRule {

val FailFastPublishUnitTestResults = Issue(
id = "FailFastPublishUnitTestResults",
title = "publish-unit-test-result-action should fail fast.",
title = "`publish-unit-test-result-action` should fail fast.",
description = """
`EnricoMi/publish-unit-test-result-action` should be configured to fail the CI when no test results are found.

Expand Down Expand Up @@ -156,7 +156,7 @@ public class FailFastActionsRule : VisitorRule {

val FailFastPeterEvansCreatePullRequest = Issue(
id = "FailFastPeterEvansCreatePullRequest",
title = "peter-evans/create-pull-request has unsafe edge cases, use `gh pr create` instead.",
title = "`peter-evans/create-pull-request` has unsafe edge cases, use `gh pr create` instead.",
description = """
Action doesn't allow fast fail, and therefore black-listed.

Expand All @@ -175,7 +175,7 @@ public class FailFastActionsRule : VisitorRule {
There's also confusion as seen [in the issue list](https://github.com/peter-evans/create-pull-request/issues?q=is%3Aissue+%22is+not+ahead+of+base%22+%22will+not+be+created%22).

The only way to notice this is by checking the logs of the action:
```
```log
Branch 'to-create' is not ahead of base 'main' and will not be created
```
and this line is not even a warning.
Expand Down
Loading