Skip to content

Bug infinate loading #645

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
yashchaud wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Bug infinate loading #645

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da6c3c6
bug: fixing infinate loading bug by adding proper exception handling
yashchaud Jun 21, 2026
f1ffb22
fix: fixed xss attack posibility with sanitization
yashchaud Jun 21, 2026
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
32 changes: 22 additions & 10 deletions frontend/src/pages/Setup.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
</p>
</div>

<div class="flex flex-col py-5 gap-3">
<div v-if="createTeam.error" class="flex flex-col py-5 gap-3">
<ErrorMessage class="text-center" :message="sanitizedError" />
</div>
<div v-else class="flex flex-col py-5 gap-3">
<LoadingIndicator class="size-5 self-center" />
<div class="text-sm text-center">Just a minute...</div>
</div>
Expand All @@ -28,21 +31,30 @@
</template>

<script setup>
import { createResource, FormControl, LoadingIndicator } from 'frappe-ui'
import { ref, computed } from 'vue'
import { computed, onMounted } from 'vue'
import { ErrorMessage, LoadingIndicator } from 'frappe-ui'
import FrappeDriveLogo from '@/components/FrappeDriveLogo.vue'
import { useRoute } from 'vue-router'
import { useStore } from 'vuex'
import { createTeam } from '@/resources/permissions'
Comment on lines 38 to 39

@greptile-apps greptile-apps Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 useStore is imported but never referenced in the component after the refactor.

Suggested change
import { useStore } from 'vuex'
import { createTeam } from '@/resources/permissions'
import { createTeam } from '@/resources/permissions'
Prompt To Fix With AI 
This is a comment left during a code review.
Path: frontend/src/pages/Setup.vue
Line: 38-39

Comment:
`useStore` is imported but never referenced in the component after the refactor.

```suggestion
import { createTeam } from '@/resources/permissions'
```

How can I resolve this? If you propose a fix, please make it concise.

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!


const route = useRoute()

createTeam.submit(
{ personal: 1 },
{
onSuccess: (data) => {
if (data) window.location.replace(route.query['redirect-to'] || '/drive')
const sanitizedError = computed(() => {
const raw = createTeam.error?.messages?.[0] ?? ''
const el = document.createElement('div')
el.textContent = raw
return el.innerHTML.replace(/&lt;(\/?strong)&gt;/g, '<$1>')
})
Comment on lines +43 to +48

@greptile-apps greptile-apps Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Bespoke HTML sanitization passed to component as trusted HTML

sanitizedError constructs a string containing real <strong> tags from a server-supplied error message, then passes it to <ErrorMessage :message="sanitizedError" />. If ErrorMessage from frappe-ui renders the message prop with v-html (rather than {{ message }}), any server-controlled string that happens to embed <strong> alongside other payloads could bypass the regex — e.g. a multi-message response or a future server change widening the allowed tags. If ErrorMessage does NOT render with v-html, the entire sanitization round-trip is wasted and <strong> tags will appear as literal text instead of bold. In either case, prefer passing createTeam.error?.messages?.[0] directly (plain text), or use a proper sanitizer like DOMPurify if HTML rendering is genuinely required.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: frontend/src/pages/Setup.vue
Line: 43-48

Comment:
**Bespoke HTML sanitization passed to component as trusted HTML**

`sanitizedError` constructs a string containing real `<strong>` tags from a server-supplied error message, then passes it to `<ErrorMessage :message="sanitizedError" />`. If `ErrorMessage` from frappe-ui renders the `message` prop with `v-html` (rather than `{{ message }}`), any server-controlled string that happens to embed `<strong>` alongside other payloads could bypass the regex — e.g. a multi-message response or a future server change widening the allowed tags. If `ErrorMessage` does NOT render with `v-html`, the entire sanitization round-trip is wasted and `<strong>` tags will appear as literal text instead of bold. In either case, prefer passing `createTeam.error?.messages?.[0]` directly (plain text), or use a proper sanitizer like DOMPurify if HTML rendering is genuinely required.

How can I resolve this? If you propose a fix, please make it concise.

@yashchaud yashchaud Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i could add DOMPurify but it is a additional Dependency and just {{}} shows html tags which looks ugly, so this is sort of a middle ground

@greptile-apps greptile-apps Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fair point. After checking, ErrorMessage does use v-html internally, so HTML rendering is real and plain text would show literal tags.

Your sanitization logic is actually sound on closer inspection: el.textContent = raw HTML-encodes everything (including any <script>, onerror=, etc.), then you selectively un-encode only <strong> tags. Since <strong> has no src/href/event attributes, there's no XSS vector here. The regex can't be bypassed because all other tags were already encoded by the textContent assignment first.

I withdraw my concern — this is a reasonable middle ground given the constraints. The implementation is safe as written.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.


onMounted(() => {
createTeam.submit(
{ personal: 1 },
{
onSuccess: (data) => {
if (data) window.location.replace(route.query['redirect-to'] || '/drive')
},
},
}
)
)
})
</script>
Loading