Flatpak#3
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Linux Flatpak packaging support: the icon build script gains GNOME-variant constants and writes ChangesLinux Flatpak Packaging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
c0d5cda to
3148711
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/desktop-build.yml:
- Around line 49-50: Replace the derogatory term "retarded" in the comment at
lines 49-50 of the desktop-build.yml workflow file with neutral, professional
language that accurately describes the behavior of electron-builder (such as
"automatically" or "inevitably"). Apply the same replacement to the other
occurrences mentioned in the review at lines 58-60 and 73-75 to ensure
consistent professional language throughout the workflow comments.
- Around line 24-27: Replace the current empty-string container configuration
approach with a `fromJSON` function combined with matrix variables that properly
use `null` values. Update the container section to use `container: ${{
fromJSON(matrix.container) }}` instead of the separate image and options fields
with empty-string defaults. Then modify the matrix strategy to include
conditional entries where non-Linux runners (such as macos-latest or
windows-latest) have their container variable set to `null`, while Linux runners
have a JSON object string containing the image configuration. This ensures
containerization is properly disabled for non-container-capable platforms rather
than attempting to set an empty string which GitHub Actions does not support.
- Around line 19-20: The container field in the desktop-build workflow is using
a mutable tag (freedesktop-25.08) which creates a supply-chain security risk in
privileged builds. Replace the mutable tag reference in the container field with
an immutable image digest by appending `@sha256`: followed by the specific digest
hash of the desired image version. This ensures the exact image version is
locked and cannot be reassigned by a registry compromise.
In `@assets/desktop-icon/build.mjs`:
- Around line 10-11: The import statements using destructuring syntax violate
the configured ESLint object-curly-spacing rule, which requires spaces inside
curly braces. Add spaces after the opening brace and before the closing brace in
all object destructuring patterns, including the imports on lines 10 and 11
(from 'child_process' and 'fs' respectively) and the destructuring pattern on
line 73. Change patterns like {spawnSync} to { spawnSync } and {mkdirSync,
readFileSync, rmSync, writeFileSync} to { mkdirSync, readFileSync, rmSync,
writeFileSync }.
In `@electron-builder.yml`:
- Around line 43-50: Replace the derogatory language in the flatpak
configuration comments with professional, neutral wording. Update the comment
explaining electron-builder's default version behavior (around the
runtimeVersion property) to use neutral language instead of "retarded", and
similarly update the comment about SVG icon handling (above the baseVersion
property) to describe the technical issue without derogatory terminology while
maintaining the explanation of why the workaround is necessary.
- Around line 1-3: The appId value in electron-builder.yml has been changed
globally from `io.github.MarkusBirkl.inutweb` to `pet.tei.inutweb` to satisfy
Flatpak D-Bus naming requirements, but this change will break existing macOS
installations (affecting NSUserDefaults and sandboxed containers) and modify the
Windows AUMID. Confirm with the team whether this breaking change to appId is
intentional for all platforms. If it is not intentional, either revert appId to
the original value and explore platform-specific Flatpak configuration options
in electron-builder.yml, or implement a migration strategy for existing users.
If the change is intentional, ensure the release notes and upgrade documentation
clearly communicate this breaking change and any necessary migration steps for
end users.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d73daea-c04a-4af3-9c67-bd197c686649
⛔ Files ignored due to path filters (2)
assets/desktop-icon/icon-linux.pngis excluded by!**/*.pngassets/desktop-icon/icon-linux.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
.github/workflows/desktop-build.ymlassets/desktop-icon/build.mjselectron-builder.ymlpackage.json
| container: ghcr.io/flathub-infra/flatpak-github-actions:freedesktop-25.08 | ||
| container-options: --privileged |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="flathub-infra/flatpak-github-actions"
tag="freedesktop-25.08"
token="$(python - <<'PY'
import json, urllib.request
u = urllib.request.urlopen("https://ghcr.io/token?scope=repository:flathub-infra/flatpak-github-actions:pull")
print(json.load(u)["token"])
PY
)"
digest="$(curl -fsSI \
-H "Authorization: Bearer ${token}" \
-H "Accept: application/vnd.oci.image.index.v1+json, application/vnd.docker.distribution.manifest.list.v2+json" \
"https://ghcr.io/v2/${repo}/manifests/${tag}" \
| tr -d '\r' \
| awk -F': ' 'tolower($1)=="docker-content-digest"{print $2}')"
echo "Resolved digest: ${digest}"Repository: teidesu/tweb
Length of output: 2638
🏁 Script executed:
# Check if the file exists and read lines around 19-20
if [ -f .github/workflows/desktop-build.yml ]; then
echo "=== File exists, showing lines 15-25 ==="
sed -n '15,25p' .github/workflows/desktop-build.yml | cat -n
else
echo "File not found"
fiRepository: teidesu/tweb
Length of output: 514
🌐 Web query:
container image pinning by digest security best practice GitHub Actions
💡 Result:
Pinning container images by digest (SHA256) is a critical security best practice in GitHub Actions to prevent supply chain attacks and ensure build reproducibility [1][2][3]. Why Pinning by Digest is Necessary Container image tags (e.g.,:latest,:v1, or:18) are mutable, meaning the underlying image content associated with a tag can be changed at any time by the publisher without notice [2][3]. Relying on tags exposes your workflow to several risks: - Supply Chain Attacks: If a registry account is compromised, an attacker can push a malicious image to an existing tag, causing your workflows to execute unauthorized code [2][3]. - Non-reproducible Builds: Because the image can change, different runs of the same workflow version may use different image layers, leading to inconsistent build results [1][2]. - Silent Compromise: There is no inherent indication or warning when the content of a tag changes, making it difficult to detect that your build environment has been altered [2]. Best Practices for Implementation - Use Immutable Digests: Always reference images using the @sha256: suffix (e.g., node@sha256:a5e0ed...) rather than relying solely on tags [1][2][4]. You can optionally keep the tag for human readability if formatted as image:tag@sha256:... [2]. - Automate Updates: Manually updating digests is error-prone. Use tools like Dependabot or Renovate to automatically monitor for new digests and open pull requests to update your workflow files [3]. - Verify Image Integrity: Where possible, use technologies like Cosign or Docker Content Trust to verify image signatures alongside digest pinning [4]. - Use Trusted Sources: Whenever possible, pull images from secure, private mirrors or registries that support scanning and policy enforcement before the images are available to CI runners [2][3]. How to Find a Digest You can identify the current digest of a container image using the following commands: - Docker CLI: docker buildx imagetools inspect <image:tag> or docker inspect --format='{{index .RepoDigests 0}}' <image:tag> [5][3]. - Third-party tools: Utilities like crane digest <image:tag> can retrieve digests without requiring a local Docker daemon [2][6][5].
Citations:
- 1: https://docs.datadoghq.com/security/code_security/iac_security/iac_rules/cicd/github/unpinned_images/
- 2: https://sisaku-security.github.io/lint/docs/rules/unpinnedimages/
- 3: https://actsense.dev/vulnerabilities/unpinned_container_image/
- 4: https://mikael.barbero.tech/blog/post/2026-03-24-stop-trusting-mutable-references/
- 5: feat: container image digest pinning in actions-lock.json with update/upgrade integration github/gh-aw#25688
- 6: https://github.com/azu/dockerfile-pin
Pin the Flatpak container image by digest.
Line 19 uses a mutable tag in a privileged job container. Pinning to a digest materially reduces CI supply-chain risk. Mutable tags can be reassigned by a registry account compromise, potentially injecting malicious code into privileged builds.
Proposed fix
- container: ghcr.io/flathub-infra/flatpak-github-actions:freedesktop-25.08
+ container: ghcr.io/flathub-infra/flatpak-github-actions:freedesktop-25.08@sha256:<resolved-digest>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/desktop-build.yml around lines 19 - 20, The container
field in the desktop-build workflow is using a mutable tag (freedesktop-25.08)
which creates a supply-chain security risk in privileged builds. Replace the
mutable tag reference in the container field with an immutable image digest by
appending `@sha256`: followed by the specific digest hash of the desired image
version. This ensures the exact image version is locked and cannot be reassigned
by a registry compromise.
Source: Linters/SAST tools
| # Ignored if empty | ||
| container: | ||
| image: ${{ matrix.container || '' }} | ||
| options: ${{ matrix.container-options || '' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does GitHub Actions officially support setting jobs.<job_id>.container.image to an empty string to disable job containers for non-Linux matrix entries?
💡 Result:
As of June 2026, GitHub Actions does not natively support disabling a job container by setting jobs.<job_id>.container.image to an empty string [1][2]. While recent updates to the workflow parser have relaxed schema validation to allow empty strings at parse time [2], the runtime behavior remains restrictive. Specifically, setting the container image to an empty string for a job will typically result in a workflow error or failure, as the runner expects a valid container image identifier when the container key is defined [1][2]. For matrix strategies that require running jobs both inside and outside of containers (such as across Linux, Windows, and macOS), the standard and supported approach is to set the entire container key to null rather than using an empty string for the image [3]. This can be achieved effectively by using a matrix variable combined with the fromJSON function [3]: jobs: test: runs-on: ${{ matrix.os }} container: ${{ fromJSON(matrix.container) }} strategy: matrix: os: [ubuntu-latest, macos-latest] container: - '{"image": "ubuntu:latest"}' # For Linux jobs - null # For non-Linux/no-container jobs This method allows you to dynamically control container usage per matrix entry, ensuring jobs on platforms that do not support Docker (like macOS or Windows) or jobs intended to run directly on the runner host can bypass containerization entirely [3].
Citations:
- 1: Allow null container actions/runner#2667
- 2: Relax container image schema and add custom validation actions/languageservices#323
- 3: https://stackoverflow.com/questions/76919426/is-it-possible-to-use-docker-for-linux-in-a-matrix-of-operating-systems
Replace empty-string container config with null to avoid runtime failures on non-Linux runners.
GitHub Actions does not support disabling containers by setting jobs.<job_id>.container.image to an empty string—this causes runtime errors. The standard approach is to use null with fromJSON and a matrix variable:
container: ${{ fromJSON(matrix.container) }}
strategy:
matrix:
include:
- os: ubuntu-latest
container: '{"image": "ubuntu:latest"}'
- os: macos-latest
container: nullThis ensures jobs on non-Linux platforms (macOS, Windows) or jobs intended to run directly on the runner bypass containerization entirely.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 26-26: unpinned image references (unpinned-images): container image may be unpinned
(unpinned-images)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/desktop-build.yml around lines 24 - 27, Replace the
current empty-string container configuration approach with a `fromJSON` function
combined with matrix variables that properly use `null` values. Update the
container section to use `container: ${{ fromJSON(matrix.container) }}` instead
of the separate image and options fields with empty-string defaults. Then modify
the matrix strategy to include conditional entries where non-Linux runners (such
as macos-latest or windows-latest) have their container variable set to `null`,
while Linux runners have a JSON object string containing the image
configuration. This ensures containerization is properly disabled for
non-container-capable platforms rather than attempting to set an empty string
which GitHub Actions does not support.
| import {spawnSync} from 'child_process'; | ||
| import {mkdirSync, readFileSync, rmSync} from 'fs'; | ||
| import {mkdirSync, readFileSync, rmSync, writeFileSync} from 'fs'; |
There was a problem hiding this comment.
Fix ESLint object-curly-spacing violations.
Line 10, Line 11, and Line 73 violate the configured spacing rule and can break lint-gated checks.
Proposed fix
-import {spawnSync} from 'child_process';
-import {mkdirSync, readFileSync, rmSync, writeFileSync} from 'fs';
+import { spawnSync } from 'child_process';
+import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
...
-const linux = buildSvg({size: GNOME_CONTENT, offset: GNOME_MARGIN, radius: GNOME_RADIUS});
+const linux = buildSvg({ size: GNOME_CONTENT, offset: GNOME_MARGIN, radius: GNOME_RADIUS });Also applies to: 73-73
🧰 Tools
🪛 ESLint
[error] 10-10: A space is required after '{'.
(object-curly-spacing)
[error] 10-10: A space is required before '}'.
(object-curly-spacing)
[error] 11-11: A space is required after '{'.
(object-curly-spacing)
[error] 11-11: A space is required before '}'.
(object-curly-spacing)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/desktop-icon/build.mjs` around lines 10 - 11, The import statements
using destructuring syntax violate the configured ESLint object-curly-spacing
rule, which requires spaces inside curly braces. Add spaces after the opening
brace and before the closing brace in all object destructuring patterns,
including the imports on lines 10 and 11 (from 'child_process' and 'fs'
respectively) and the destructuring pattern on line 73. Change patterns like
{spawnSync} to { spawnSync } and {mkdirSync, readFileSync, rmSync,
writeFileSync} to { mkdirSync, readFileSync, rmSync, writeFileSync }.
Source: Linters/SAST tools
| # Flatpak requires appId to be a valid D-Bus bus name. A valid D-Bus bus name | ||
| # contains at least 3 segments (intended for reverse dns notation). | ||
| appId: pet.tei.inutweb |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In electron-builder v26, does changing top-level appId affect macOS bundle identifier and Windows app identity, and is there a Flatpak-only appId override?
💡 Result:
Yes, changing the top-level appId property in electron-builder (including in v26) does affect both the macOS bundle identifier and the Windows application identity [1][2][3]. For macOS, the appId serves as the CFBundleIdentifier [1]. It is strongly recommended to set this explicitly; changing it after an initial release can break existing user data paths, such as NSUserDefaults and sandboxed containers [1]. For Windows, the appId is used as the Application User Model ID (AUMID) for the NSIS target [1][4]. Note that this functionality is specific to NSIS and is not supported by Squirrel.Windows [1][4]. Regarding Flatpak, electron-builder does not provide a specific top-level "appId override" key just for Flatpak. While you can configure Flatpak-specific options under the flatpak configuration block, the appId itself is generally handled globally [5]. Flatpak identifiers (often formatted like org.domain.appname) are typically defined within the Flatpak-specific manifest or configuration settings, and you should ensure your application identity aligns with the requirements of the Flatpak ecosystem [6][7].
Citations:
- 1: https://www.electron.build/mac
- 2: https://www.electron.build/configuration
- 3: https://www.electron.build/app-builder-lib.interface.platformspecificbuildoptions
- 4: https://www.electron.build/win
- 5: https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/configuration.ts
- 6: https://www.electron.build/flatpak
- 7: https://docs.flatpak.org/en/latest/electron.html
Confirm this is an intentional breaking change; appId migration affects macOS user data and Windows app identity.
Changing the top-level appId from io.github.MarkusBirkl.inutweb to pet.tei.inutweb will break existing installations on macOS (invalidates NSUserDefaults paths and sandboxed containers) and change the Windows AUMID for NSIS. There is no Flatpak-only override for appId; the global change applies to all platforms. Confirm this is intentional before release, especially if existing users exist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron-builder.yml` around lines 1 - 3, The appId value in
electron-builder.yml has been changed globally from
`io.github.MarkusBirkl.inutweb` to `pet.tei.inutweb` to satisfy Flatpak D-Bus
naming requirements, but this change will break existing macOS installations
(affecting NSUserDefaults and sandboxed containers) and modify the Windows
AUMID. Confirm with the team whether this breaking change to appId is
intentional for all platforms. If it is not intentional, either revert appId to
the original value and explore platform-specific Flatpak configuration options
in electron-builder.yml, or implement a migration strategy for existing users.
If the change is intentional, ensure the release notes and upgrade documentation
clearly communicate this breaking change and any necessary migration steps for
end users.
3148711 to
cdb2792
Compare
Also generate a scalable SVG icon bc it's the new standard.
cdb2792 to
7ae1435
Compare
I'll add a proper manifest later (instead of relying on electron-builder) so that everything looks less ugly. But uh, this at least works already.
Summary by CodeRabbit
Release Notes
New Features
Chores
Build/Packaging