Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 3 additions & 3 deletions unraid-ui/src/components/common/accordion/Accordion.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
AccordionRoot,
AccordionTrigger,
} from '@/components/ui/accordion';
import { computed, ref, watch } from 'vue';
import { ref, watch } from 'vue';

export interface AccordionItemData {
value: string;
Expand All @@ -31,7 +31,7 @@ const props = withDefaults(defineProps<AccordionProps>(), {
});

const emit = defineEmits<{
'update:modelValue': [value: string | string[]];
'update:modelValue': [value: string | string[] | undefined];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Synchronize externally cleared model values.

Now that undefined is a valid update payload, the watcher at Line [42] must also assign undefined. Otherwise, changing a controlled modelValue from a selected value to undefined leaves openValue stale and the accordion remains open.

   (val) => {
-    if (val !== undefined) openValue.value = val;
+    openValue.value = val;
   }

Also applies to: 52-52

🤖 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 `@unraid-ui/src/components/common/accordion/Accordion.vue` at line 34, Update
the watcher around openValue to propagate externally cleared modelValue values,
including undefined, instead of ignoring them. Ensure both the single-value and
array-value branches assign the incoming undefined so openValue is cleared and
the accordion closes.

}>();

const openValue = ref<string | string[] | undefined>(props.modelValue ?? props.defaultValue);
Expand All @@ -49,7 +49,7 @@ function isItemOpen(itemValue: string): boolean {
return openValue.value === itemValue;
}

function handleUpdate(value: string | string[]) {
function handleUpdate(value: string | string[] | undefined) {
openValue.value = value;
emit('update:modelValue', value);
}
Expand Down
83 changes: 83 additions & 0 deletions web/__test__/components/ApiStatus.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* ApiStatus Component Test Coverage
*
* Regression coverage for the "wrong csrf_token" bug: the on-mount status
* check must use the page-global csrf_token when the server store has not yet
* hydrated its own `csrf` value, otherwise it sends a blank token and both
* fails the status read and logs a red error in syslog.
*/

import { flushPromises, mount } from '@vue/test-utils';

import { createTestingPinia } from '@pinia/testing';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

const mockWebguiUnraidApiCommand = vi.fn();

vi.mock('~/composables/services/webgui', () => ({
WebguiUnraidApiCommand: (...args: unknown[]) => mockWebguiUnraidApiCommand(...args),
}));

const mockServerStore = { csrf: '' };
vi.mock('~/store/server', () => ({
useServerStore: () => mockServerStore,
}));

import ApiStatus from '~/components/ApiStatus/ApiStatus.vue';

const mountComponent = () =>
mount(ApiStatus, {
global: {
plugins: [createTestingPinia({ createSpy: vi.fn })],
},
});

describe('ApiStatus.vue', () => {
beforeEach(() => {
vi.clearAllMocks();
mockServerStore.csrf = '';
mockWebguiUnraidApiCommand.mockResolvedValue({ result: 'API is running' });
globalThis.csrf_token = 'global-token-123';
});

afterEach(() => {
globalThis.csrf_token = '';
});

it('sends the page-global csrf_token on mount when the store has not hydrated', async () => {
mountComponent();
await flushPromises();

expect(mockWebguiUnraidApiCommand).toHaveBeenCalledWith({
csrf_token: 'global-token-123',
command: 'status',
});
});

it('never sends a blank csrf_token on mount', async () => {
mountComponent();
await flushPromises();

const payload = mockWebguiUnraidApiCommand.mock.calls[0]?.[0];
expect(payload.csrf_token).toBeTruthy();
});

it('prefers the store csrf value once it is populated', async () => {
mockServerStore.csrf = 'store-token-456';
mountComponent();
await flushPromises();

expect(mockWebguiUnraidApiCommand).toHaveBeenCalledWith({
csrf_token: 'store-token-456',
command: 'status',
});
});

it('renders Running when the status result reports the service is running', async () => {
const wrapper = mountComponent();
await flushPromises();

expect(wrapper.text()).toContain('Running');
expect(wrapper.text()).not.toContain('Not Running');
});
});
55 changes: 55 additions & 0 deletions web/__test__/composables/request.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* request composable — global CSRF header attachment
*
* The webGUI CSRF gate rejects same-origin POSTs that lack a valid token. The
* shared wretch instance must attach the page-global csrf_token as an
* `x-csrf-token` header automatically (and only for same-origin requests).
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { request } from '~/composables/services/request';

const okResponse = () =>
new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } });

describe('request csrf attachment', () => {
let fetchMock: ReturnType<typeof vi.fn>;

beforeEach(() => {
fetchMock = vi.fn().mockResolvedValue(okResponse());
vi.stubGlobal('fetch', fetchMock);
globalThis.csrf_token = 'token-abc';
});

afterEach(() => {
vi.unstubAllGlobals();
globalThis.csrf_token = '';
});

const headerFromCall = (index = 0) => {
const opts = fetchMock.mock.calls[index]?.[1] ?? {};
return new Headers(opts.headers).get('x-csrf-token');
};

it('attaches the page-global token on same-origin requests', async () => {
await request.url('/plugins/dynamix.my.servers/include/unraid-api.php').post();
expect(headerFromCall()).toBe('token-abc');
});

it('attaches the token on same-origin GET requests too', async () => {
await request.url('/plugins/dynamix.my.servers/data/server-state.php').get();
expect(headerFromCall()).toBe('token-abc');
});

it('does not attach the token to cross-origin requests', async () => {
await request.url('https://wanip4.unraid.net/').get();
expect(headerFromCall()).toBeNull();
});

it('attaches no header when the page global is unset', async () => {
globalThis.csrf_token = '';
await request.url('/webGui/include/Notify.php').post();
expect(headerFromCall()).toBeNull();
});
});
12 changes: 10 additions & 2 deletions web/src/components/ApiStatus/ApiStatus.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ import { useServerStore } from '~/store/server';

const serverStore = useServerStore();

/**
* The webGUI embeds `csrf_token` as a page global and validates every plugin
* request against it. Prefer that synchronously-available value; the server
* store's `csrf` is only populated after async state hydration, so relying on
* it in onMounted sends a blank token and trips a "wrong csrf_token" log error.
*/
const resolveCsrfToken = () => serverStore.csrf || globalThis.csrf_token || '';

const apiStatus = ref<string>('');
const isRunning = ref<boolean>(false);
const isLoading = ref<boolean>(false);
Expand All @@ -18,7 +26,7 @@ const checkStatus = async () => {
statusMessage.value = '';
try {
const response = await WebguiUnraidApiCommand({
csrf_token: serverStore.csrf,
csrf_token: resolveCsrfToken(),
command: 'status',
});

Expand Down Expand Up @@ -53,7 +61,7 @@ const restartApi = async () => {

try {
const response = await WebguiUnraidApiCommand({
csrf_token: serverStore.csrf,
csrf_token: resolveCsrfToken(),
command: 'restart',
});

Expand Down
21 changes: 21 additions & 0 deletions web/src/composables/services/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,32 @@ import queryString from 'wretch/addons/queryString';

import { useErrorsStore } from '~/store/errors';

/**
* The webGUI CSRF gate (local_prepend.php) validates same-origin POSTs against
* `$var['csrf_token']`, accepting the token via an `x-csrf-token` header for
* XHR/fetch callers. jQuery callers get this for free via `$.ajaxPrefilter`, and
* the Apollo client already sets the header itself — attach it here so every
* fetch-based webGUI request is covered too, rather than each caller having to
* remember to pass the token. Restricted to same-origin requests so the token is
* never sent to external hosts.
*/
const isSameOriginRequest = (url: string): boolean => {
try {
return new URL(url, globalThis.location?.href).origin === globalThis.location?.origin;
} catch {
return true;
}
};
Comment on lines +8 to +23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the target file and inspect nearby context.
ast-grep outline web/src/composables/services/request.ts --view expanded || true
echo '--- FILE ---'
cat -n web/src/composables/services/request.ts | sed -n '1,220p'

echo '--- SEARCH CALLERS / URL SHAPES ---'
rg -n "isSameOriginRequest|csrf_token|x-csrf-token|new URL\(" web/src -S

Repository: unraid/api

Length of output: 7783


🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files 'web/src/composables/services/request.ts' 'web/src/**' | sed -n '1,20p'
echo '---'
wc -l web/src/composables/services/request.ts
echo '---'
cat -n web/src/composables/services/request.ts | sed -n '1,220p'

Repository: unraid/api

Length of output: 3333


🏁 Script executed:

node - <<'JS'
const cases = [
  ['relative with base href', ['/foo', 'https://example.com/app/']],
  ['absolute', ['https://api.example.com/x', 'https://example.com/app/']],
  ['relative no base', ['/foo', undefined]],
  ['bad url', ['http://[bad', 'https://example.com/app/']],
  ['empty string', ['', 'https://example.com/app/']],
  ['non-string', [null, 'https://example.com/app/']],
];
for (const [label, [u, b]] of cases) {
  try {
    const r = new URL(u, b);
    console.log(label, '=>', r.origin, r.href);
  } catch (e) {
    console.log(label, '=> THROW', e.message);
  }
}
JS

Repository: unraid/api

Length of output: 472


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "from ['\"].*composables/services/request['\"]|from ['\"]~/composables/services/request['\"]|request\." web/src -S

Repository: unraid/api

Length of output: 1525


Fail closed on parse errors. isSameOriginRequest already blocks absolute external URLs, but the catch still treats malformed or SSR/non-browser URLs as same-origin and attaches x-csrf-token. Return false there to keep the “never sent to external hosts” rule intact.

🤖 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 `@web/src/composables/services/request.ts` around lines 8 - 23, Update
isSameOriginRequest so its catch branch returns false when URL parsing or
location resolution fails. Preserve the existing same-origin comparison for
successfully parsed URLs, ensuring CSRF tokens are not attached when the request
origin cannot be verified.


export const request = wretch()
.addon(formData)
.addon(formUrl)
.addon(queryString)
.errorType('json')
.defer((w, url) => {
const token = globalThis.csrf_token;
return token && isSameOriginRequest(url) ? w.headers({ 'x-csrf-token': token }) : w;
})
.resolve((response) => {
return response
.error('Error', (error) => {
Expand Down
Loading