fix: minimal fix for airgap sample trust and factory URL source comparison#1567
Merged
Conversation
…rison
Four targeted changes to fix workspace source comparison and airgap sample
trust for factory flows like:
/load-factory?url=https%3A%2F%2Fregistry.devfile.io%2F...&storageType=per-user
1. Preferences/helpers.ts — automatically trust airgap sample URLs
(matching /dashboard/api/airgap-sample/devfile/download) so the factory
flow proceeds without requiring an explicit trust grant. Minor refactor:
extract urlString before early-return guards.
2. ImportFromGit/helpers.ts — rename REVISION import to REVISION_ATTR
(both are 'revision', so no functional change; aligns with the canonical
constant name used in buildFactoryParams.ts).
3. buildFactoryParams.ts — apply decodeURIComponent() in getSourceUrl() so
factoryParams.sourceUrl matches workspace.source. The factory hash
double-encodes airgap URLs (%253F → %3F after first URLSearchParams decode),
while workspace.source applies a second decode via URLSearchParams.get()
(%3F → ?). Without this normalization the two sides differ and
CheckExistingWorkspaces always creates a new workspace.
4. workspace-adapter/index.ts — replace split('&') + split('=')[1] with
URLSearchParams.get('url') to extract the URL from the DevWorkspace
annotation. The old approach truncated the value at the second '=' sign,
breaking URLs whose query parameters contain '=' characters (e.g.
base64-encoded values). URLSearchParams.get() also applies the same
percent-decoding as point 3, keeping both sides consistent.
Assisted-by: Claude Sonnet 4.6
Signed-off-by: Oleksii Orel <oorel@redhat.com>
Contributor
|
Click here to review and test in web IDE: |
"Sourse" is a misspelling of "Source". Rename the two local variables devfileSourseStr and devfileSourse to devfileSourceStr and devfileSource. No functional change. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com>
|
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1567 (linux/amd64, linux/arm64) kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1567", name: che-dashboard}]}}]" |
1 similar comment
|
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1567 (linux/amd64, linux/arm64) kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1567", name: che-dashboard}]}}]" |
Contributor
Author
|
/retest |
Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com>
|
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1567 (linux/amd64, linux/arm64) kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1567", name: che-dashboard}]}}]" |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1567 +/- ##
==========================================
- Coverage 92.51% 92.51% -0.01%
==========================================
Files 563 563
Lines 56189 56227 +38
Branches 4258 4263 +5
==========================================
+ Hits 51984 52019 +35
- Misses 4155 4158 +3
Partials 50 50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…W-9659)
The GitRepoFormGroup.getSource() method parsed the DevWorkspace factory
annotation with split('&') + split('=') and then reconstructed the URL by
appending every other factory param (che-editor, storageType, …).
For airgap sample URLs this produced a malformed "Git repo URL" like:
"…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user"
Two bugs in the old parser:
1. split('=')[1] truncated the URL value at the second '=' sign.
2. All non-url params were re-joined and appended to the URL, adding
factory orchestration params that have no place in the Git repo URL.
Fix: replace the parser with URLSearchParams.get('url').
URLSearchParams.get() correctly handles '=' inside values (e.g. ?id=foo)
and decodes percent-encoded characters (%3F → ?, %3D → =) so the URL shown
in the Overview tab matches the clean devfile download URL.
Add regression test: airgap sample URL with che-editor and storageType in
the factory params — verifies that only the decoded 'url' value appears and
factory params are never appended.
Also stage a CSS line-wrap format fix in SamplesList/Gallery/Card.
Assisted-by: Claude Sonnet 4.6
Signed-off-by: Oleksii Orel <oorel@redhat.com>
|
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1567 (linux/amd64, linux/arm64) kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1567", name: che-dashboard}]}}]" |
Contributor
|
/retest |
…CRW-9659)
The GitRepoFormGroup.getSource() method used split('=')[1] to extract the
'url' value from the factory annotation params string. For airgap sample URLs
whose query string contains '=' characters (e.g. %3Fid%3Dnodejs-express,
decoded by URLSearchParams to ?id=nodejs-express) this truncated the value at
the second '=' and then appended other factory params with '?' as separator,
producing a malformed double-'?' URL:
"…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user"
^ missing =nodejs-express ^ wrong separator
Fix: use URLSearchParams.get('url') to extract the full URL value.
URLSearchParams handles '=' inside values correctly and decodes percent-encoded
characters (%3F→?, %3D→=). The remaining factory params (editor-image,
storageType, etc.) are still appended to represent the full workspace config;
the separator is chosen based on whether the URL already has a '?'.
Add regression test that verifies:
- No double '?' in the resulting URL
- Decoded query value (id=nodejs-express) is preserved
- Factory params are appended with '&', not a second '?'
Also commit a CSS line-wrap format fix in SamplesList/Gallery/Card.
Assisted-by: Claude Sonnet 4.6
Signed-off-by: Oleksii Orel <oorel@redhat.com>
|
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1567 (linux/amd64, linux/arm64) kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1567", name: che-dashboard}]}}]" |
Contributor
|
/retest |
Contributor
|
@SkorikSergey could you please check the fix with DS image quay.io/redhat-user-workloads/devspaces-tenant/devspaces/dashboard-rhel9:on-pr-6cf673cd9af66b5d337ea87b32d61759a8e3ee28 |
Contributor
Works as expected on "online" DS 3.28.0 with this PR dashboard image. With factory URLs and samples. |
svor
approved these changes
May 15, 2026
SkorikSergey
approved these changes
May 15, 2026
|
@SkorikSergey: changing LGTM is restricted to collaborators DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: olexii4, SkorikSergey, svor The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
olexii4
added a commit
that referenced
this pull request
May 15, 2026
…rison (#1567) * fix: minimal fix for airgap sample trust and factory URL source comparison Four targeted changes to fix workspace source comparison and airgap sample trust for factory flows like: /load-factory?url=https%3A%2F%2Fregistry.devfile.io%2F...&storageType=per-user 1. Preferences/helpers.ts — automatically trust airgap sample URLs (matching /dashboard/api/airgap-sample/devfile/download) so the factory flow proceeds without requiring an explicit trust grant. Minor refactor: extract urlString before early-return guards. 2. ImportFromGit/helpers.ts — rename REVISION import to REVISION_ATTR (both are 'revision', so no functional change; aligns with the canonical constant name used in buildFactoryParams.ts). 3. buildFactoryParams.ts — apply decodeURIComponent() in getSourceUrl() so factoryParams.sourceUrl matches workspace.source. The factory hash double-encodes airgap URLs (%253F → %3F after first URLSearchParams decode), while workspace.source applies a second decode via URLSearchParams.get() (%3F → ?). Without this normalization the two sides differ and CheckExistingWorkspaces always creates a new workspace. 4. workspace-adapter/index.ts — replace split('&') + split('=')[1] with URLSearchParams.get('url') to extract the URL from the DevWorkspace annotation. The old approach truncated the value at the second '=' sign, breaking URLs whose query parameters contain '=' characters (e.g. base64-encoded values). URLSearchParams.get() also applies the same percent-decoding as point 3, keeping both sides consistent. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * fix(typo): rename devfileSourse → devfileSource in workspace-adapter "Sourse" is a misspelling of "Source". Rename the two local variables devfileSourseStr and devfileSourse to devfileSourceStr and devfileSource. No functional change. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * chore(license): add jsbn@0.1.1 to dev dependency excludes Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * fix: GitRepo component appends factory params to the Git repo URL (CRW-9659) The GitRepoFormGroup.getSource() method parsed the DevWorkspace factory annotation with split('&') + split('=') and then reconstructed the URL by appending every other factory param (che-editor, storageType, …). For airgap sample URLs this produced a malformed "Git repo URL" like: "…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user" Two bugs in the old parser: 1. split('=')[1] truncated the URL value at the second '=' sign. 2. All non-url params were re-joined and appended to the URL, adding factory orchestration params that have no place in the Git repo URL. Fix: replace the parser with URLSearchParams.get('url'). URLSearchParams.get() correctly handles '=' inside values (e.g. ?id=foo) and decodes percent-encoded characters (%3F → ?, %3D → =) so the URL shown in the Overview tab matches the clean devfile download URL. Add regression test: airgap sample URL with che-editor and storageType in the factory params — verifies that only the decoded 'url' value appears and factory params are never appended. Also stage a CSS line-wrap format fix in SamplesList/Gallery/Card. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> * fix: GitRepo URL truncated and double-'?' for airgap factory params (CRW-9659) The GitRepoFormGroup.getSource() method used split('=')[1] to extract the 'url' value from the factory annotation params string. For airgap sample URLs whose query string contains '=' characters (e.g. %3Fid%3Dnodejs-express, decoded by URLSearchParams to ?id=nodejs-express) this truncated the value at the second '=' and then appended other factory params with '?' as separator, producing a malformed double-'?' URL: "…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user" ^ missing =nodejs-express ^ wrong separator Fix: use URLSearchParams.get('url') to extract the full URL value. URLSearchParams handles '=' inside values correctly and decodes percent-encoded characters (%3F→?, %3D→=). The remaining factory params (editor-image, storageType, etc.) are still appended to represent the full workspace config; the separator is chosen based on whether the URL already has a '?'. Add regression test that verifies: - No double '?' in the resulting URL - Decoded query value (id=nodejs-express) is preserved - Factory params are appended with '&', not a second '?' Also commit a CSS line-wrap format fix in SamplesList/Gallery/Card. Assisted-by: Claude Sonnet 4.6 Signed-off-by: Oleksii Orel <oorel@redhat.com> --------- Signed-off-by: Oleksii Orel <oorel@redhat.com>
svor
pushed a commit
that referenced
this pull request
May 16, 2026
…rison (#1567) (#1568) * fix: minimal fix for airgap sample trust and factory URL source comparison Four targeted changes to fix workspace source comparison and airgap sample trust for factory flows like: /load-factory?url=https%3A%2F%2Fregistry.devfile.io%2F...&storageType=per-user 1. Preferences/helpers.ts — automatically trust airgap sample URLs (matching /dashboard/api/airgap-sample/devfile/download) so the factory flow proceeds without requiring an explicit trust grant. Minor refactor: extract urlString before early-return guards. 2. ImportFromGit/helpers.ts — rename REVISION import to REVISION_ATTR (both are 'revision', so no functional change; aligns with the canonical constant name used in buildFactoryParams.ts). 3. buildFactoryParams.ts — apply decodeURIComponent() in getSourceUrl() so factoryParams.sourceUrl matches workspace.source. The factory hash double-encodes airgap URLs (%253F → %3F after first URLSearchParams decode), while workspace.source applies a second decode via URLSearchParams.get() (%3F → ?). Without this normalization the two sides differ and CheckExistingWorkspaces always creates a new workspace. 4. workspace-adapter/index.ts — replace split('&') + split('=')[1] with URLSearchParams.get('url') to extract the URL from the DevWorkspace annotation. The old approach truncated the value at the second '=' sign, breaking URLs whose query parameters contain '=' characters (e.g. base64-encoded values). URLSearchParams.get() also applies the same percent-decoding as point 3, keeping both sides consistent. Assisted-by: Claude Sonnet 4.6 * fix(typo): rename devfileSourse → devfileSource in workspace-adapter "Sourse" is a misspelling of "Source". Rename the two local variables devfileSourseStr and devfileSourse to devfileSourceStr and devfileSource. No functional change. Assisted-by: Claude Sonnet 4.6 * chore(license): add jsbn@0.1.1 to dev dependency excludes Assisted-by: Claude Sonnet 4.6 * fix: GitRepo component appends factory params to the Git repo URL (CRW-9659) The GitRepoFormGroup.getSource() method parsed the DevWorkspace factory annotation with split('&') + split('=') and then reconstructed the URL by appending every other factory param (che-editor, storageType, …). For airgap sample URLs this produced a malformed "Git repo URL" like: "…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user" Two bugs in the old parser: 1. split('=')[1] truncated the URL value at the second '=' sign. 2. All non-url params were re-joined and appended to the URL, adding factory orchestration params that have no place in the Git repo URL. Fix: replace the parser with URLSearchParams.get('url'). URLSearchParams.get() correctly handles '=' inside values (e.g. ?id=foo) and decodes percent-encoded characters (%3F → ?, %3D → =) so the URL shown in the Overview tab matches the clean devfile download URL. Add regression test: airgap sample URL with che-editor and storageType in the factory params — verifies that only the decoded 'url' value appears and factory params are never appended. Also stage a CSS line-wrap format fix in SamplesList/Gallery/Card. Assisted-by: Claude Sonnet 4.6 * fix: GitRepo URL truncated and double-'?' for airgap factory params (CRW-9659) The GitRepoFormGroup.getSource() method used split('=')[1] to extract the 'url' value from the factory annotation params string. For airgap sample URLs whose query string contains '=' characters (e.g. %3Fid%3Dnodejs-express, decoded by URLSearchParams to ?id=nodejs-express) this truncated the value at the second '=' and then appended other factory params with '?' as separator, producing a malformed double-'?' URL: "…/download?id?che-editor=che-incubator/che-code/latest&storageType=per-user" ^ missing =nodejs-express ^ wrong separator Fix: use URLSearchParams.get('url') to extract the full URL value. URLSearchParams handles '=' inside values correctly and decodes percent-encoded characters (%3F→?, %3D→=). The remaining factory params (editor-image, storageType, etc.) are still appended to represent the full workspace config; the separator is chosen based on whether the URL already has a '?'. Add regression test that verifies: - No double '?' in the resulting URL - Decoded query value (id=nodejs-express) is preserved - Factory params are appended with '&', not a second '?' Also commit a CSS line-wrap format fix in SamplesList/Gallery/Card. Assisted-by: Claude Sonnet 4.6 --------- Signed-off-by: Oleksii Orel <oorel@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes two related issues that caused the "Create New" workspace policy to be ignored
when creating workspaces from devfile registry samples or airgap samples:
Airgap sample trust gate — airgap sample URLs were not automatically trusted,
causing the untrusted-source modal to block the factory flow.
Source URL comparison mismatch —
CheckExistingWorkspacescomparesworkspace.source(read from the DevWorkspace annotation) againstfactoryParams.sourceUrl(derived from the current factory URL). Two bugs causedthese values to differ, so the step always created a new workspace regardless of
the "Create New" setting:
Double-encoding in airgap URLs. The factory hash encodes the inner devfile
download URL twice (
%253F→%3Fafter the firstURLSearchParamsdecode).workspace.sourceapplied a second decode viaURLSearchParams.get()(%3F→?),while
factoryParams.sourceUrlkept the single-encoded form. Result:Value truncation on
=in URL parameters. The old annotation parser usedsplit('&')+split('=')[1]to extract theurlvalue. For any URL whosequery string contains an
=sign (e.g.?id=nodejs-express),split('=')[1]returned only the fragment up to the second
=, silently truncating the value.Screenshot/screencast of this PR
What issues does this PR fix or reference?
fixes https://redhat.atlassian.net/browse/CRW-9659
Is it tested? How?
Release Notes
Docs PR