Skip to content
Open
Show file tree
Hide file tree
Changes from all 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';

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

const mockWebguiUnraidApiCommand = vi.fn();

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

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

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