Skip to content

chore(deps-dev): bump the jest group across 1 directory with 4 updates #5341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dependabot wants to merge 9 commits into
base: unstable
Choose a base branch
from
Open

chore(deps-dev): bump the jest group across 1 directory with 4 updates #5341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cd75994
chore(deps-dev): bump the jest group across 1 directory with 4 updates
dependabot[bot] Nov 5, 2025
cd13113
use updated jest methods for jest 30 toHave* -> toHaveBeen*
nucleogenesis Nov 7, 2025
7ef1e63
use updated jest methods for jest 30 toThrowError -> toThrow
nucleogenesis Nov 7, 2025
e0ae516
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Nov 7, 2025
27f80f3
fixing incorrect Error types between spec & source for changes.js
nucleogenesis Dec 8, 2025
a823fe8
fix mis-rebased test
nucleogenesis Dec 9, 2025
dc2e17f
toBeCalled -> toHaveBeeen...
nucleogenesis Dec 9, 2025
1c3ce5b
fix mising toString mock in accoutnsMain
nucleogenesis Dec 9, 2025
21cafae
Claude: mocking window.location better and managing nextParam in a mo…
nucleogenesis Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@
showPrivacyPolicy() {
this.$router.push({ query: { showPolicy: policies.PRIVACY } });
},
navigate(path) {
// Extracted for easier testing
window.location.assign(path);

Check failure

Code scanning / CodeQL

Client-side cross-site scripting High

Cross-site scripting vulnerability due to
user-provided value
Loading
.

Copilot Autofix

AI about 12 hours ago

General Fix:
The vulnerability must be fixed by ensuring the next parameter (from the query string) cannot facilitate cross-site scripting or open redirect attacks. Only allow navigation to safe, trusted URLs. The safest practice is to allow only relative internal paths or, even stricter, to whitelist possible destinations. Arbitrary full URLs or JavaScript schemes must never be allowed.

Detailed Fix:
Update the nextParam computed property (or submit method) to permit only valid, relative paths—beginning with / and not containing any protocol (to prohibit javascript:, //, or http(s)://). If next does not pass this validation, it should be ignored or fallback to a known safe page. We can use a simple regular expression or URL parsing to verify this. Changes should be made in AccountsMain.vue (lines 155-158 and/or in the logic around line 183 in submit).

Implementation Requirements:

  • Add a function to validate (sanitize) the next parameter: ensure it is a relative path within the application, e.g., /foo, /bar/baz?x=1, etc.
  • Use this function when retrieving nextParam so it cannot return an unsafe value.
  • This can be implemented in the computed block or as a method.
  • No external dependencies required—vanilla JS suffices.
Suggested changeset 1
contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue b/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
--- a/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
+++ b/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
@@ -154,7 +154,8 @@
       }),
       nextParam() {
         const params = new URLSearchParams(window.location.search.substring(1));
-        return params.get('next');
+        const next = params.get('next');
+        return this.sanitizeNextPath(next);
       },
     },
     methods: {
@@ -196,6 +197,24 @@
         }
         return Promise.resolve();
       },
+
+      /**
+       * Only allow safe, local, relative paths for redirection.
+       * Disallows protocol-relative, external, or javascript: URLs.
+       */
+      sanitizeNextPath(next) {
+        if (
+          typeof next === 'string' &&
+          next.length > 0 &&
+          next.startsWith('/') &&
+          !next.startsWith('//') &&
+          !next.includes('://')
+        ) {
+          return next;
+        }
+        // Fallback: unsafe or missing next param
+        return null;
+      },
     },
     $trs: {
       kolibriStudio: 'Kolibri Studio',
EOF
@@ -154,7 +154,8 @@
}),
nextParam() {
const params = new URLSearchParams(window.location.search.substring(1));
return params.get('next');
const next = params.get('next');
return this.sanitizeNextPath(next);
},
},
methods: {
@@ -196,6 +197,24 @@
}
return Promise.resolve();
},

/**
* Only allow safe, local, relative paths for redirection.
* Disallows protocol-relative, external, or javascript: URLs.
*/
sanitizeNextPath(next) {
if (
typeof next === 'string' &&
next.length > 0 &&
next.startsWith('/') &&
!next.startsWith('//') &&
!next.includes('://')
) {
return next;
}
// Fallback: unsafe or missing next param
return null;
},
},
$trs: {
kolibriStudio: 'Kolibri Studio',
Copilot is powered by AI and may make mistakes. Always verify output.

Check warning

Code scanning / CodeQL

Client-side URL redirect Medium

Untrusted URL redirection depends on a
user-provided value
Loading
.

Copilot Autofix

AI about 12 hours ago

To fix the open redirect issue, we must ensure that the next parameter extracted from the query string cannot be used to redirect users to arbitrary, potentially malicious, locations. The best approach is to only allow redirection to safe, internal paths. We can do this by only permitting relative paths that start with a single / and do not contain a protocol (://) or start with // (protocol-relative). We'll add a validation method that checks the supplied path, and only performs the redirect if it is deemed safe; otherwise, we fall back to a known-safe default (such as window.Urls.channels()). We will implement this validation inside the Vue component, ensuring minimal change to feature behavior.

The edits should be within contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue, particularly in the method that handles the redirect (navigate), the computed property for nextParam, and the logic that determines the redirect target. We'll add a method isSafeRedirectPath for validation.

Suggested changeset 1
contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue b/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
--- a/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
+++ b/contentcuration/contentcuration/frontend/accounts/pages/AccountsMain.vue
@@ -154,7 +154,11 @@
       }),
       nextParam() {
         const params = new URLSearchParams(window.location.search.substring(1));
-        return params.get('next');
+        const param = params.get('next');
+        if (param && this.isSafeRedirectPath(param)) {
+          return param;
+        }
+        return null;
       },
     },
     methods: {
@@ -166,7 +170,7 @@
         this.$router.push({ query: { showPolicy: policies.PRIVACY } });
       },
       navigate(path) {
-        // Extracted for easier testing
+        // Only redirect to internal, validated paths
         window.location.assign(path);
       },
       submit() {
@@ -180,7 +184,7 @@
             .then(() => {
               this.loginFailedOffline = false;
               this.loginFailed = false;
-              const path = this.nextParam || window.Urls.channels();
+              const path = this.nextParam ? this.nextParam : window.Urls.channels();
               this.navigate(path);
             })
             .catch(err => {
@@ -197,6 +201,20 @@
         return Promise.resolve();
       },
     },
+    /**
+     * Accepts only safe internal paths:
+     * - Starts with a single "/" (not "//")
+     * - Does not contain ":" before a "?" or "#" (no protocol-relative or absolute URLs)
+     * - Optional: Could be tightened to match a whitelist
+     */
+    isSafeRedirectPath(path) {
+      // Must start with /, not with //
+      if (typeof path !== "string") return false;
+      if (!path.startsWith('/') || path.startsWith('//')) return false;
+      // Prevent "http:/", "http://", etc.
+      if (/^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(path)) return false;
+      return true;
+    },
     $trs: {
       kolibriStudio: 'Kolibri Studio',
       passwordLabel: 'Password',
EOF
@@ -154,7 +154,11 @@
}),
nextParam() {
const params = new URLSearchParams(window.location.search.substring(1));
return params.get('next');
const param = params.get('next');
if (param && this.isSafeRedirectPath(param)) {
return param;
}
return null;
},
},
methods: {
@@ -166,7 +170,7 @@
this.$router.push({ query: { showPolicy: policies.PRIVACY } });
},
navigate(path) {
// Extracted for easier testing
// Only redirect to internal, validated paths
window.location.assign(path);
},
submit() {
@@ -180,7 +184,7 @@
.then(() => {
this.loginFailedOffline = false;
this.loginFailed = false;
const path = this.nextParam || window.Urls.channels();
const path = this.nextParam ? this.nextParam : window.Urls.channels();
this.navigate(path);
})
.catch(err => {
@@ -197,6 +201,20 @@
return Promise.resolve();
},
},
/**
* Accepts only safe internal paths:
* - Starts with a single "/" (not "//")
* - Does not contain ":" before a "?" or "#" (no protocol-relative or absolute URLs)
* - Optional: Could be tightened to match a whitelist
*/
isSafeRedirectPath(path) {
// Must start with /, not with //
if (typeof path !== "string") return false;
if (!path.startsWith('/') || path.startsWith('//')) return false;
// Prevent "http:/", "http://", etc.
if (/^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(path)) return false;
return true;
},
$trs: {
kolibriStudio: 'Kolibri Studio',
passwordLabel: 'Password',
Copilot is powered by AI and may make mistakes. Always verify output.
},
submit() {
if (this.$refs.form.validate()) {
this.busy = true;
Expand All @@ -176,7 +180,8 @@
.then(() => {
this.loginFailedOffline = false;
this.loginFailed = false;
window.location.assign(this.nextParam || window.Urls.channels());
const path = this.nextParam || window.Urls.channels();
this.navigate(path);
})
.catch(err => {
this.busy = false;
Expand Down
62 changes: 38 additions & 24 deletions contentcuration/contentcuration/frontend/accounts/pages/__tests__/accountsMain.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render, screen, waitFor } from '@testing-library/vue';
import { render, screen, waitFor, within } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import VueRouter from 'vue-router';
import AccountsMain from '../AccountsMain.vue';
Expand Down Expand Up @@ -28,30 +28,43 @@ const createRouter = () => {
function makeWrapper({ loginMock = jest.fn(), online = true, nextParam = null } = {}) {
const router = createRouter();

delete window.location;
window.location = {
...originalLocation,
search: nextParam ? `?next=${nextParam}` : '',
assign: jest.fn(),
// Mock window.location.search using JSDOM's history API
const searchString = nextParam ? `?next=${nextParam}` : '';
window.history.pushState({}, '', `/${searchString}`);

// Create a spy for navigate before rendering
const navigateSpy = jest.fn();
const OriginalAccountsMain = AccountsMain;
Copy link
Member

@nucleogenesis nucleogenesis Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the witchcraft that Claude came up with to accommodate the need to use the path in the next parameter to redirect.

I tried doing it w/ Vue Router and ran into redundant navigation errors. I tried mocking window.location all sorts of different ways but always ended up running into the issue of it being read-only.

I kind of hate everything about this because it feels like it should be done differently if this is how the tests have to be written, but my attempts to do it differently fizzled out in annoying ways where when I got it working in the browser, tests failed and vice versa until I asked Claude a second time to "figure it out please" and let them run with it.

It works and tests pass now anyway


// Patch the component to use our spy
const PatchedAccountsMain = {
...OriginalAccountsMain,
methods: {
...OriginalAccountsMain.methods,
navigate: navigateSpy,
},
};

return {
...render(AccountsMain, {
routes: router,
stubs: ['PolicyModals'],
mocks: {
$store: {
state: {
connection: {
online,
},
const wrapper = render(PatchedAccountsMain, {
routes: router,
stubs: ['PolicyModals'],
mocks: {
$store: {
state: {
connection: {
online,
},
dispatch: loginMock,
},
dispatch: loginMock,
},
}),
},
});

return {
...wrapper,
router,
loginMock,
navigateSpy,
};
}

Expand All @@ -60,11 +73,12 @@ describe('AccountsMain', () => {

beforeEach(() => {
user = userEvent.setup();
jest.clearAllMocks();
});

afterEach(() => {
window.location = originalLocation;
// Reset history
window.history.pushState({}, '', '/');
jest.restoreAllMocks();
});

it('should render sign-in form with email, password fields and sign in button', () => {
Expand Down Expand Up @@ -104,15 +118,15 @@ describe('AccountsMain', () => {

it('should redirect to channels page after successful login', async () => {
const loginMock = jest.fn().mockResolvedValue();
makeWrapper({ loginMock });
const { navigateSpy } = makeWrapper({ loginMock });

await user.type(screen.getByLabelText(/email/i), '[email protected]');
await user.type(screen.getByLabelText(/password/i), 'testpassword');
await user.click(screen.getByRole('button', { name: /sign in/i }));

// User is redirected to channels page
await waitFor(() => {
expect(window.location.assign).toHaveBeenCalledWith('/channels/');
expect(navigateSpy).toHaveBeenCalledWith('/channels/');
});
});

Expand All @@ -125,15 +139,15 @@ describe('AccountsMain', () => {
it('should redirect to next URL when provided after successful login', async () => {
const loginMock = jest.fn().mockResolvedValue();
const nextUrl = '/protected-page/';
makeWrapper({ loginMock, nextParam: nextUrl });
const { navigateSpy } = makeWrapper({ loginMock, nextParam: nextUrl });

await user.type(screen.getByLabelText(/email/i), '[email protected]');
await user.type(screen.getByLabelText(/password/i), 'testpassword');
await user.click(screen.getByRole('button', { name: /sign in/i }));

// User is redirected to next URL
await waitFor(() => {
expect(window.location.assign).toHaveBeenCalledWith(nextUrl);
expect(navigateSpy).toHaveBeenCalledWith(nextUrl);
});
});

Expand Down
18 changes: 9 additions & 9 deletions ...contentcuration/frontend/channelEdit/components/AssessmentEditor/AssessmentEditor.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ describe('AssessmentEditor', () => {

it('emits delete item event with a correct key', () => {
expect(listeners.deleteItem).toHaveBeenCalledWith(ITEM2);
expect(listeners.updateItems).toBeCalledTimes(1);
expect(listeners.updateItems).toHaveBeenCalledTimes(1);
});

it('emits update item events with updated order of items after the deleted item', () => {
Expand All @@ -288,7 +288,7 @@ describe('AssessmentEditor', () => {
order: 2,
},
]);
expect(listeners.updateItems).toBeCalledTimes(1);
expect(listeners.updateItems).toHaveBeenCalledTimes(1);
});
});

Expand All @@ -300,7 +300,7 @@ describe('AssessmentEditor', () => {
});

it('emits add item event with a new item with a correct order', () => {
expect(listeners.addItem).toBeCalledWith({
expect(listeners.addItem).toHaveBeenCalledWith({
contentnode: NODE_ID,
question: '',
type: AssessmentItemTypes.SINGLE_SELECTION,
Expand Down Expand Up @@ -330,7 +330,7 @@ describe('AssessmentEditor', () => {
order: 4,
},
]);
expect(listeners.updateItems).toBeCalledTimes(1);
expect(listeners.updateItems).toHaveBeenCalledTimes(1);
});
});

Expand All @@ -351,7 +351,7 @@ describe('AssessmentEditor', () => {
order: 2,
[DELAYED_VALIDATION]: true,
});
expect(listeners.addItem).toBeCalledTimes(1);
expect(listeners.addItem).toHaveBeenCalledTimes(1);
});

it('emits update item events with updated order of items below the new item', () => {
Expand All @@ -373,7 +373,7 @@ describe('AssessmentEditor', () => {
order: 4,
},
]);
expect(listeners.updateItems).toBeCalledTimes(1);
expect(listeners.updateItems).toHaveBeenCalledTimes(1);
});
});

Expand All @@ -395,7 +395,7 @@ describe('AssessmentEditor', () => {
order: 1,
},
]);
expect(listeners.updateItems).toBeCalledTimes(1);
expect(listeners.updateItems).toHaveBeenCalledTimes(1);
});
});

Expand All @@ -417,7 +417,7 @@ describe('AssessmentEditor', () => {
order: 1,
},
]);
expect(listeners.updateItems).toBeCalledTimes(1);
expect(listeners.updateItems).toHaveBeenCalledTimes(1);
});
});

Expand All @@ -427,7 +427,7 @@ describe('AssessmentEditor', () => {
});

it('emits add item event with a new item with a correct order', () => {
expect(listeners.addItem).toBeCalledWith({
expect(listeners.addItem).toHaveBeenCalledWith({
contentnode: NODE_ID,
question: '',
type: AssessmentItemTypes.SINGLE_SELECTION,
Expand Down
Loading
Loading