Validate SteamOS app identifiers#601
Conversation
Greptile SummaryThis PR adds a
Confidence Score: 3/5The core validation logic is sound, but the two log statements that still reference raw Two
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[build / ship called] --> B[requireAppId]
B --> C{appId empty?}
C -- yes --> D[throw: requires appId]
C -- no --> E{matches APP_ID_PATTERN?}
E -- no --> F[throw: must be reverse-DNS]
E -- yes --> G[return trimmed appId]
G --> H[ctx.log — ⚠️ still uses config.appId]
H --> I{build or ship?}
I -- build --> J[write steamos-flatpak-plan.json\nusing validated appId]
I -- ship --> K{dryRun?}
K -- yes --> L[return dry-run id]
K -- no --> M[return id & Flathub URL\nusing validated appId]
Reviews (1): Last reviewed commit: "Validate SteamOS app identifiers" | Re-trigger Greptile |
| label: 'SteamOS / Steam Deck (Desktop Mode / Flatpak)', | ||
| async build(ctx, config) { | ||
| const appId = requireAppId(config); | ||
| ctx.log(`flatpak-builder · appId=${config.appId}`); |
There was a problem hiding this comment.
Both
ctx.log calls still reference config.appId (raw input) instead of the validated, trimmed appId returned by requireAppId. If a caller passes an appId with surrounding whitespace, the log emits the untrimmed string while every downstream artifact (JSON plan, outputArtifact, ship id, Flathub URL) uses the trimmed value — making the log misleading. The same stale reference exists in ship() at line 64.
| ctx.log(`flatpak-builder · appId=${config.appId}`); | |
| ctx.log(`flatpak-builder · appId=${appId}`); |
| async ship(ctx, config) { | ||
| const appId = requireAppId(config); | ||
| const dest = config.distribution === 'flathub' ? 'Flathub PR' : `self-hosted (${config.selfHosted?.uploadTo})`; | ||
| ctx.log(`publish ${config.appId}@${ctx.version} → ${dest}`); |
There was a problem hiding this comment.
Same stale reference in
ship(): the log uses the raw config.appId while the returned id and url already use the validated appId. When the inputs differ (e.g. due to whitespace), the log disagrees with the emitted ship record.
| ctx.log(`publish ${config.appId}@${ctx.version} → ${dest}`); | |
| ctx.log(`publish ${appId}@${ctx.version} → ${dest}`); |
| selfHosted?: { uploadTo: 'github-pages' | 'cdn' | 's3' }; | ||
| } | ||
|
|
||
| const APP_ID_PATTERN = /^[A-Za-z][A-Za-z0-9-]*(\.[A-Za-z][A-Za-z0-9-]*)+$/; |
There was a problem hiding this comment.
The segment character class
[A-Za-z0-9-] omits underscores, but the Flatpak app-ID spec (which follows D-Bus naming) explicitly permits underscores in segments — e.g. org.kde.Plasma_Shell. Valid identifiers like com.acme.my_app would be rejected, potentially blocking users with legitimate Flatpak IDs.
| const APP_ID_PATTERN = /^[A-Za-z][A-Za-z0-9-]*(\.[A-Za-z][A-Za-z0-9-]*)+$/; | |
| const APP_ID_PATTERN = /^[A-Za-z][A-Za-z0-9_-]*(\.[A-Za-z][A-Za-z0-9_-]*)+$/; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes #600.
Changes:
Validation: