content: maintain docs about yubikey situation - #31
Conversation
|
Hey @Stensel8, looks like you forgot something! The following images in
Please convert them before merging. Install sudo pacman -S libavifThen batch-convert all images in cd static/images
for f in *.png *.jpg *.jpeg; do
[ -f "$f" ] && avifenc -q 80 -s 6 "$f" "${f%.*}.avif" && rm "$f"
done |
There was a problem hiding this comment.
Pull request overview
This PR updates the YubiKey security documentation to reflect abandoning FIDO2 LUKS unlock on this hardware (due to a USB timing race), and documents a reliable alternative using pam-u2f for sudo and GNOME screen unlock. It also adds a GitHub Actions workflow to enforce AVIF-only image assets and introduces new screenshots for the docs.
Changes:
- Update EN/NL YubiKey docs with new findings (systemd 259 status,
FIDO_ERR_RXrace) and a fullpam-u2fsetup guide. - Add new YubiKey screenshots under
static/images/to support the updated docs. - Replace the “auto-convert images to AVIF” workflow with a “check & comment/fail on non-AVIF” workflow; add a YubiKey entry to Known Issues.
Reviewed changes
Copilot reviewed 5 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
static/images/yubikey-sudo-test.png |
Adds screenshot for sudo touch prompt (currently PNG). |
static/images/yubikey-sudo-config.png |
Adds screenshot of PAM sudo config (currently PNG). |
static/images/yubikey-polkit.png |
Adds screenshot of GNOME polkit prompt (currently PNG). |
static/images/yubikey-gdm-password-config.png |
Adds screenshot of PAM gdm-password config (currently PNG). |
content/docs/security/yubikey.md |
EN doc expanded with pam-u2f setup, polkit, lock screen, boot flow, and screenshots. |
content/docs/security/yubikey.nl.md |
NL equivalent of the expanded YubiKey doc. |
content/docs/known-issues.md |
Adds an active issue entry documenting the FIDO2 LUKS USB timing race. |
content/docs/known-issues.nl.md |
NL equivalent of the new known issue entry. |
.github/workflows/convert-images.yml |
Reworked to check for non-AVIF images, comment on PRs, and fail with annotations. |
Comments suppressed due to low confidence (2)
.github/workflows/convert-images.yml:38
count=$(... | wc -l)will include leading spaces fromwc, so[ "$count" -gt 0 ]can error with “integer expression expected”. Strip whitespace (e.g.| tr -d ' ') or usecount=$(...; echo $(( ... )))/mapfileto compute the count robustly.
count=$(find static/images -type f \( -iname "*.png" -o -iname "*.jpg" -o -iname "*.jpeg" \) | wc -l)
if [ "$count" -gt 0 ]; then
echo "found=true" >> "$GITHUB_OUTPUT"
else
echo "found=false" >> "$GITHUB_OUTPUT"
echo "All images are AVIF."
.github/workflows/convert-images.yml:76
- The PR comment step always creates a new comment via
issues.createComment, so re-runs / pushes will spam duplicates. Consider searching existing comments for a unique marker and updating it (or deleting/replacing), or rely solely on workflow annotations to avoid noisy PR threads.
- name: Post PR comment
if: steps.check.outputs.found == 'true' && github.event_name == 'pull_request'
uses: actions/github-script@v7
env:
FILES: ${{ steps.check.outputs.files }}
ACTOR: ${{ github.event.pull_request.user.login }}
with:
script: |
const files = process.env.FILES.trim().split('\n').map(f => `- \`${f}\``).join('\n');
const actor = process.env.ACTOR;
const body = [
`Hey @${actor}, looks like you forgot something!`,
'',
'The following images in `static/images/` are not in AVIF format:',
files,
'',
'Please convert them before merging. Install `avifenc` first:',
'```bash',
'sudo pacman -S libavif',
'```',
'',
'Then batch-convert all images in `static/images/`:',
'```bash',
'cd static/images',
'for f in *.png *.jpg *.jpeg; do',
' [ -f "$f" ] && avifenc -q 80 -s 6 "$f" "${f%.*}.avif" && rm "$f"',
'done',
'```',
].join('\n');
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.payload.pull_request.number,
body,
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|  | ||
|
|
||
| Test without closing the current terminal first: | ||
|
|
||
| ```bash | ||
| sudo echo test | ||
| # "Please touch the FIDO authenticator." → touch → done | ||
| ``` | ||
|
|
||
|  | ||
|
|
There was a problem hiding this comment.
These markdown image links point to .avif files, but in static/images/ the corresponding screenshots currently exist as .png (and there is no .avif variant). This will break the rendered docs and fail the image-format checks; please convert the images to AVIF (preferred) or update the references to match the committed filenames.
|  | ||
|
|
||
| Test eerst zonder de huidige terminal te sluiten: | ||
|
|
||
| ```bash | ||
| sudo echo test | ||
| # "Please touch the FIDO authenticator." → aanraken → klaar | ||
| ``` | ||
|
|
||
|  | ||
|
|
There was a problem hiding this comment.
Deze markdown image links verwijzen naar .avif bestanden, maar in static/images/ bestaan de bijbehorende screenshots nu als .png (en er is geen .avif variant). Dit breekt de gerenderde docs en laat de image-format checks falen; converteer de images naar AVIF (voorkeur) of update de referenties zodat ze overeenkomen met de aanwezige bestandsnamen.
|
|
||
| De `cue` tekstprompt verschijnt ook in de grafische dialoog. De YubiKey aanraken authenticeert zonder wachtwoord te hoeven typen. Zonder YubiKey ingeplugd valt hij terug op wachtwoord. | ||
|
|
||
|  |
There was a problem hiding this comment.
Deze afbeelding gebruikt /images/yubikey-polkit.avif, maar static/images/ bevat momenteel yubikey-polkit.png en geen .avif bestand. Converteer de screenshot naar AVIF (voorkeur) en houd de .avif link aan, of update de link zodat hij overeenkomt met de echte assetnaam.
|  | |
|  |
|  | ||
|
|
||
| Vergrendel het scherm met `Super+L` en raak de YubiKey aan om te ontgrendelen. | ||
|
|
||
|  |
There was a problem hiding this comment.
Deze image links verwijzen naar .avif bestanden (yubikey-gdm-password-config.avif, yubikey-lockscreen.avif), maar static/images/ bevat momenteel .png varianten en geen .avif bestanden. Dit breekt de gerenderde docs en laat de image-format checks falen; converteer naar AVIF en behoud de .avif links (voorkeur) of update de links naar de bestaande assets.
|  | |
| Vergrendel het scherm met `Super+L` en raak de YubiKey aan om te ontgrendelen. | |
|  | |
|  | |
| Vergrendel het scherm met `Super+L` en raak de YubiKey aan om te ontgrendelen. | |
|  |
|
|
||
| The `cue` text prompt does appear in the graphical dialog as well. Touching the YubiKey authenticates without needing to type a password. Without the key plugged in, it falls back to password as usual. | ||
|
|
||
|  |
There was a problem hiding this comment.
This image reference uses /images/yubikey-polkit.avif, but static/images/ currently contains yubikey-polkit.png and no .avif file. Please convert the screenshot to AVIF (preferred) and keep the .avif link, or update the link to match the actual asset filename.
|  | |
|  |
|  | ||
|
|
||
| Lock the screen with `Super+L` and touch the YubiKey to unlock. | ||
|
|
||
|  |
There was a problem hiding this comment.
These image references point to .avif files (yubikey-gdm-password-config.avif, yubikey-lockscreen.avif), but static/images/ currently contains .png variants and no .avif files. This will break the rendered docs and fail the image-format checks; please convert to AVIF and keep the .avif references (preferred) or update the links to match the committed assets.
|  | |
| Lock the screen with `Super+L` and touch the YubiKey to unlock. | |
|  | |
|  | |
| Lock the screen with `Super+L` and touch the YubiKey to unlock. | |
|  |
I want to improve my PR automations. The intended goal is to notify the user if something went wrong.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name: Update PR checklist | ||
| runs-on: ubuntu-latest | ||
| if: always() | ||
| needs: [pr-title, bilingual, image-format, link-check] | ||
| steps: | ||
| - uses: actions/github-script@v7 | ||
| env: | ||
| RESULT_PR_TITLE: ${{ needs.pr-title.result }} | ||
| RESULT_BILINGUAL: ${{ needs.bilingual.result }} | ||
| RESULT_IMAGE_FORMAT: ${{ needs.image-format.result }} | ||
| RESULT_LINK_CHECK: ${{ needs.link-check.result }} | ||
| with: | ||
| script: | | ||
| const { data: pr } = await github.rest.pulls.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: context.payload.pull_request.number, | ||
| }); | ||
|
|
||
| let body = pr.body || ''; | ||
|
|
||
| const setCheck = (keyword, passed) => { | ||
| body = body.replace( | ||
| new RegExp(`- \\[[ xX]\\] (.*${keyword}.*)`, 'i'), | ||
| `- [${passed ? 'x' : ' '}] $1` | ||
| ); | ||
| }; | ||
|
|
||
| setCheck('PR title follows', process.env.RESULT_PR_TITLE === 'success'); | ||
| setCheck('Both EN and NL', process.env.RESULT_BILINGUAL === 'success'); | ||
| setCheck('Media is in AVIF', process.env.RESULT_IMAGE_FORMAT === 'success'); | ||
| setCheck('No broken image', process.env.RESULT_LINK_CHECK === 'success'); | ||
|
|
||
| await github.rest.pulls.update({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number: context.payload.pull_request.number, | ||
| body, | ||
| }); |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
In general, you fix this by adding an explicit permissions section either at the workflow root (to apply to all jobs) or per job, ensuring each job only has the minimal permissions it requires for GITHUB_TOKEN. Here, the code fragment shows several jobs: some only read code or upload/download artifacts, and the update-checklist job reads and updates the PR body. We should add a workflow-level default of contents: read (safe and commonly needed) and then override per job where necessary, notably granting pull-requests: write for the update-checklist job while leaving other jobs with read-only permissions.
Concretely, in .github/workflows/pr-checks.yml:
- Add a top-level
permissions:block after theon:section to set safe defaults, e.g.:contents: read(for checking out code, etc.).
- Leave most jobs to inherit this default, since they only read code or work with artifacts.
- For the
update-checklistjob (lines 196–235), add apermissions:block under the job definition specifying:pull-requests: write(to update the PR body).- Optionally
contents: readif you want to keep consistency, though it isn’t strictly necessary for that job.
This does not change any existing functional behavior besides tightening token scope; all steps, including the PR body update, will continue to work, now with explicitly documented and minimized permissions.
| @@ -4,6 +4,9 @@ | ||
| pull_request: | ||
| branches: [main, development] | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
|
|
||
| # ── 1. PR title follows conventional commits ──────────────────────────────── | ||
| @@ -198,6 +201,8 @@ | ||
| runs-on: ubuntu-latest | ||
| if: always() | ||
| needs: [pr-title, bilingual, image-format, link-check] | ||
| permissions: | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/github-script@v7 | ||
| env: |
Summary
Maintaining some docs, because I changed my Yubikey setup and wanted to share it.
Type of change
feat— new page or featurefix— bug fix (broken link, incorrect command, layout issue)content— update or improve existing contentdocs— changes to CONTRIBUTING, README, or meta documentationchore— maintenance (dependencies, config, CI/CD)refactor— restructuring without content changesstyle— formatting, whitespace, typosChecklist
fix: correct nmcli command in eduroam guide)/images/*.avifall exist instatic/images/)hugo server