Scorecard findings only showed up in the weekly workflow run against
`master`, well after the change had merged. The `lint.sh scorecard`
subcommand runs the four file based checks up front, so lefthook catches
them at commit time and reviewdog annotates them on the pull request.
The checks run against a `git checkout-index` export rather than the
working tree. Scanning the tree directly walks every `node_modules` and
does not finish; the export takes about 15 seconds. Only checks that
lost points are reported, as `scorecard` also warns about things it does
not penalize, such as the job level `contents` write the contributors
workflow needs to push the card.
`scorecard` and `jq` are now required tools. CI agents need `scorecard`
installed for the reviewdog runner.
---------
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
`typos` aborts with `SIGABRT` when `std::fs::canonicalize` fails for an
entry the walker handed it: `report_result` reports the error and
substitutes `PathBuf::default()`, and `ConfigEngine::policy` then panics
on that empty path with `` `walk()` should be called first ``
(crate-ci/typos#1444, fix proposed in crate-ci/typos#1577). reviewdog
runs every runner in `.reviewdog.yml` concurrently in the same checkout,
so a file removed by another linter mid-walk is enough to trigger it, as
it was on #13100.
The wrapper prefixed every stderr line with a position so none of them
were discarded, which turned one crash into one finding per line: the
failing check carried 81 summary entries and 30 annotations, all of them
fragments of a single backtrace.
Prefix only the first line and let the rest match a `%C` continuation,
as the `golangci` runner already does. reviewdog puts the `%m` capture
of the `%E` line in `Diagnostic.Message` and the whole entry in
`Diagnostic.OriginalOutput`, which `service/github/check.go` sends as an
annotation's `raw_details`, so the crash becomes a single finding whose
reason is the first line and whose full backtrace stays intact one click
away. Ordinary typo findings are unaffected because an `%E` match
terminates the preceding entry.
The `%C%.%#` line is required rather than cosmetic: with `%E` alone the
entry is never terminated and reviewdog emits nothing at all.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
The `typos` runner intermittently dies with `signal: aborted (core
dumped)` on the Buildkite agents, and reviewdog reports only `typos
failed with zero findings: The command itself failed`. reviewdog
discards a runner's stderr along with any stdout line that does not
match the `errorformat`, so whatever `typos` wrote on its way out never
reaches the build log, leaving nothing to diagnose.
Route the runner through `.buildkite/steps/lint.sh typos`, matching the
existing `shellcheck` subcommand. The wrapper captures stderr and, on
any exit code other than `0` (clean) or `2` (typos found), re-emits each
line as an `errorformat` matching finding against `.reviewdog.yml`. A
`SIGABRT` now surfaces as `typos failed (exit 134): <reason>` instead of
as an empty failure.
Cap `--threads` at `4` in both the wrapper and the lefthook job. `typos`
defaults to one thread per logical CPU and runs alongside five other
linters in the same step, which makes an in-process allocator abort the
most likely explanation for the crash. The whole repository still scans
in under a tenth of a second.
`docs/` ships a prettier configuration but nothing ever ran it, so
formatting drift was only caught when somebody thought to look. Wire it
into the two gates that already exist: lefthook fixes and re-stages the
docs files a commit touches, and reviewdog reports whatever is left on
the pull request.
Prettier has no machine readable output of its own, so it runs as the
only enabled rule of an ESLint configuration and reports through
`rdjson`, the same way the web package already does. That earns a
message naming the replacement, a range covering the offending word
rather than the whole line, and a suggestion the reviewer can commit,
none of which a diff of a rewritten tree can carry. It also leaves the
working tree alone, which matters because runners execute in parallel.
The `@eslint/*` packages are parsers rather than linters here: ESLint
has to build an AST before a rule can run, even one that works on the
raw source text, and that parse is the one thing prettier itself never
needs. It is also the only place the two disagree. SCSS gets through the
CSS parser only in tolerant mode, and the NDJSON test result file has to
be excluded because no JSON parser will accept it. Those exclusions live
in the ESLint configuration because prettier has no configuration file
option for ignores, and `.prettierignore` goes with them: every
remaining entry it carried named an extension ESLint never reaches.
Both lefthook jobs now lint `{staged_files}` rather than the whole
package. `stage_fixed` only ever stages the files lefthook filtered, so
a job that fixes everything leaves its repairs to files outside the
commit sitting in the working tree, surfacing only once the commit has
already finished. Scoping the command to the same set it can stage keeps
the two in step and stops a commit rewriting files it was never asked to
touch.
Correct the two integration guides the new configuration finds: emphasis
markers prettier normalises, which nothing had ever checked.
Rename `.prettierrc.yaml` to `prettier.config.mjs` so it matches the web
package, and drop the `format` script, which nothing calls now that
lefthook, reviewdog and the aggregate entrypoint all go through ESLint.
The linting step grows a `pnpm -C docs install` for the same reason it
already has one for `web`.
Both `lint` scripts take optional paths and fall back to the whole tree,
which is what lets lefthook hand them the staged set. Drop `--ext` while
passing, since flat configuration takes its file selection from the
configuration itself and the value given was a glob rather than the
extension list the flag expects. Replace `cd web &&` with `pnpm -C web
exec`, since `pnpm -C <dir> <binary>` resolves a binary only through
`exec` when it is not also a script of that package.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
The import convention is four groups: the standard library, general
third party packages, the company packages under `authelia.com` and
`github.com/authelia`, and the project itself. `goimports` cannot
express this because `local-prefixes` describes a single trailing group
no matter how many prefixes are listed, so it tolerates the existing
layout but merges the company group into the third party group whenever
it formats a file, which `goimports-reviser` then has to undo.
`gci` is enabled to enforce it as its sections accept an ordered list,
with `prefix` covering both company prefixes and `localmodule` resolving
the project. It is bundled with `golangci-lint` so it replaces a
separately installed binary, and it runs after `goimports` so the two no
longer disagree about the result.
Equivalence was established by scrambling the import block of all six
hundred and thirteen files which have one and formatting the result with
each tool: the output is byte identical, including the twenty six
generated files both correctly decline to touch. `goimports-reviser` is
therefore removed from the reviewdog runners, the lint step, the git
hooks and the contributor documentation.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
Wire goimports-reviser into the three existing lint paths (pre-commit
via lefthook, CI review comments via reviewdog, and the manual runner in
`.buildkite/steps/lint.sh`) so import ordering, grouping, and
unused-import removal are enforced consistently across local and CI
workflows.
The runner is configured with `-rm-unused -format -company-prefixes
authelia.com,github.com/authelia`, grouping first-party Authelia
packages into their own block after third-party imports. `-set-alias` is
intentionally omitted; imports work without an alias, and we do not want
one added automatically.
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
Introduce ShellCheck as an enforced lint step so shell scripts are held
to the same strictness as the Go, YAML, and JavaScript surfaces, and
take the opportunity to normalize the repository's shell hygiene while
the tooling is being put in place. A new .shellcheckrc keeps the default
ShellCheck rule set enabled and opts in to require-variable-braces
(SC2250) to enforce the ${VAR} style repo-wide. The tool is wired into
the local pre-commit path via lefthook (with file_types filtering on
text/x-shellscript and text/x-sh so only libmagic-detected shell files
are passed in, and --format=gcc so the output is clickable from the
IDE), into the Buildkite 🐕🦺 Linting step via a new shellcheck
subcommand on .buildkite/steps/lint.sh that discovers every tracked
shell script through git ls-files and a shebang scan, and into reviewdog
via .reviewdog.yml using the same lint.sh subcommand with --format=gcc
so findings annotate pull requests inline.
Along with the wiring this commit clears the ShellCheck baseline against
the current tree. Every bare $VAR reference was rewritten to ${VAR} so
SC2250 lands on an already-clean repository. Unquoted expansions and
command substitutions (SC2086 and SC2046) were quoted across the
Buildkite steps, hooks, pipeline.sh, setup.sh, redis and samba
entrypoints, and grypescans.sh; read calls gained -r (SC2162), cd calls
got || exit guards (SC2164), declare-and-assign in bootstrap.sh was
split to preserve return values (SC2155), indirect $? checks in setup.sh
were rewritten as if ! cmd (SC2181), repeated redirect blocks in samba
init.sh were collapsed into { ... } > file (SC2129), an unused LDOMAIN
variable was removed (SC2034), the runtime source in healthcheck.sh was
marked with a # shellcheck source=/dev/null directive (SC1091), and
framework-consumed variables in authelia-fb-rc.d gained a file-level
disable directive. Two suite entrypoints that used bash-only syntax
under #!/bin/sh had their shebangs bumped to #!/usr/bin/env bash, two
dev-host gen.sh scripts had their /bin/bash absolute paths swapped for
/usr/bin/env bash so they pick up Homebrew bash on developer machines,
and grypescans.sh had its split-string grypeCmd rewritten as a proper
bash array so the intentional word-splitting no longer needed an inline
disable.
---------
Signed-off-by: Amir Zarrinkafsh <3339418+nightah@users.noreply.github.com>
Co-authored-by: James Elliott <james-d-elliott@users.noreply.github.com>
Add the typos (crate-ci/typos) spell checker with en-US locale
configuration and integrate it into the lefthook pre-commit hooks and
reviewdog runner. Fix all existing spelling errors across documentation,
Go source, test files, configuration templates, and locale files
including en-GB to en-US normalization.
Closes#11607.
* build(deps): update dependency eslint to v9
Adapt configuration for ESLint v9 flat configuration and remove unused eslint plugins. Renamed and adapted web linting configuration to ESM variants and renamed extensions to .mjs equivalents.
* fix(web): update eslint configuration for v9
* fix(web): eslint flat config with vite-plugin-checker
* ci(reviewdog): update eslint runner to use repo version
Closes#9126.
---------
Signed-off-by: Amir Zarrinkafsh <nightah@me.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Amir Zarrinkafsh <nightah@me.com>
* ci(lefthook): replace husky with lefthook
This change removes husky in favour of [LeftHook](https://lefthook.dev/) along with the following:
* Remove unused pre-commit configuration
* Included trufflehog as a standard linter locally
* ci(lefthook): adjust gitleaks to use detect
* ci(lefthook): adjust lefthook for yamllint
* ci(lefthook): skip pnpm prepare for dockerfile.coverage
* ci(lefthook): add yamllint to lefthook
* ci(lefthook): posix compliant check tools job
* ci(lefthook): yamllint success feedback like golangci-lint
* ci(lefthook): remove gitleaks
* ci(lefthook): ignore lefthook-local
* ci(lefthook): add trufflehog
* ci(lefthook): eslint success feedback like golangci-lint
Signed-off-by: Amir Zarrinkafsh <nightah@me.com>
This change implements yamllint and adjusts all yaml files to abide by our linting setup. This excludes config.template.yml as this will be done in an alternate commit.
Amir Zarrinkafsh