Skip to content

VPS-96/Allowed renaming of resources in edit view #413

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
kmck133 wants to merge 5 commits into
base: master
Choose a base branch
from
+171 −27
Open

VPS-96/Allowed renaming of resources in edit view #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47f0e18
Allowed renaming of resources in edit view
kmck133 May 26, 2026
10b3fbd
Fixed resourse page code format
kmck133 May 26, 2026
c45bb80
Text in sidebar now truncates if file name overflows
kmck133 Jun 3, 2026
121e598
Merge branch 'master' into VPS-96/change-resource-name
kmck133 Jun 3, 2026
bab4def
Prettier + lint
kmck133 Jun 3, 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
28 changes: 28 additions & 0 deletions backend/src/routes/api/files.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,34 @@ router.post("/upload", upload.array("files"), async (req, res) => {
}
});

/**
* @route PATCH /api/files/:fileId
* @desc Update the name of a stored file
*/
router.patch("/:fileId", async (req, res) => {
try {
const { fileId } = req.params;
const { name } = req.body;
if (!name || !name.trim()) {
return res.status(400).json({ error: "Name is required" });
}
Comment on lines +155 to +158
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against non-string name to avoid a 500.

A truthy non-string name (e.g. { "name": 123 }) passes the !name check, then name.trim() throws a TypeError, which is caught and returned as a generic 500 instead of a 400.

🛡️ Proposed fix 
-    const { name } = req.body;
-    if (!name || !name.trim()) {
+    const { name } = req.body;
+    if (typeof name !== "string" || !name.trim()) {
       return res.status(400).json({ error: "Name is required" });
     }
🤖 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 `@backend/src/routes/api/files.js` around lines 155 - 158, Validate that
req.body.name is a string before calling name.trim(): change the guard around
"const { name } = req.body" to check typeof name !== 'string' || !name.trim()
and return res.status(400).json({ error: "Name is required" }) for non-string or
empty values so name.trim() cannot throw; reference the existing "name"
variable, the name.trim() call, and the res.status(...) return to locate and
update the check.

if (name.trim().length > 255) {
return res
.status(400)
.json({ error: "Name must be 255 characters or fewer" });
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const meta = await StoredFile.findById(fileId);
if (!meta) return res.status(404).json({ error: "File not found" });
meta.name = name.trim();
await meta.save();
const file = meta.toObject();
delete file.gridFsId;
return res.json(file);
} catch (err) {
return res.status(500).json({ error: err.message });
}
});
Comment on lines +152 to +174
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for any scenario/file ownership or role checks in the files routes and middleware
rg -nP -C3 '(ownerUid|uploaderUid|scenarioId|authoriz|permission|forbidden|403)' backend/src/routes/api/files.js
fd -t f . backend/src/middleware --exec rg -nP -C3 '(role|owner|permission|scenario|403|forbidden)' {}
# Check how collections routes guard scenario access for comparison
fd -t f 'collections.js' backend/src --exec rg -nP -C3 '(owner|uid|role|permission|403)' {}

Repository: UoaWDCC/VPS

Length of output: 5875

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show the PATCH /:fileId handler and surrounding code in backend/src/routes/api/files.js
sed -n '1,260p' backend/src/routes/api/files.js | nl -ba | sed -n '120,220p'

# 2) Confirm auth middleware wiring in that file
rg -n --hidden "router\.use\(|auth|scenarioAuth|dashboardAuth|permission|role|403|forbidden|owner|uid" backend/src/routes/api/files.js

# 3) Find any authorization helper used for fileId-based routes (rename/delete) in this route file
rg -n --hidden "patch\\(\"/:fileId\"|delete\\(\"/:fileId\"|StoredFile\\.findById\\(|findById\\(fileId\\)|fileId\\)" backend/src/routes/api/files.js

# 4) Check if any middleware elsewhere enforces ownership for StoredFile routes
#    (search for StoredFile access-control patterns)
fd -t f . backend/src/middleware backend/src/routes/api backend/src/db/daos backend/src/db/models --max-depth 6 2>/dev/null | head -n 2000 >/tmp/filelist.txt
rg -n --hidden "(StoredFile|gridFsId|uploaderUid|scenarioId|groupId|ownerUid|can[A-Z]|hasAccess|authorize|permission|role)" $(cat /tmp/filelist.txt) || true

# 5) Specifically locate scenario/file ownership checks in middleware and list them
rg -n --hidden "(assert.*Scenario|assert.*Group|scenarioAuth|dashboardAuth|access|ownerId)" backend/src/middleware backend/src/routes/api | head -n 200 || true

Repository: UoaWDCC/VPS

Length of output: 98

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show backend/src/routes/api/files.js around the PATCH handler using cat -n (nl not available)
sed -n '1,260p' backend/src/routes/api/files.js | cat -n | sed -n '120,220p'

# 2) Confirm auth middleware wiring in that file
rg -n --hidden "router\.use\(|auth|scenarioAuth|dashboardAuth|permission|role|403|forbidden|owner|uid" backend/src/routes/api/files.js || true

# 3) Locate the patch/delete routes and where fileId is used
rg -n --hidden "router\.(patch|delete)\(\"/:fileId\"|StoredFile\.findById|findById\(\s*fileId|uploaderUid|scenarioId|groupId" backend/src/routes/api/files.js || true

# 4) Search for any middleware/utility used by files.js for access control
#    (start with scenarioAuth/dashboardAuth and access checks; then also search for StoredFile ownership checks)
fd -t f backend/src/middleware backend/src/routes/api backend/src/db/daos backend/src/db/models --max-depth 6 2>/dev/null > /tmp/vps_files.txt
rg -n --hidden "(scenarioAuth|dashboardAuth|accessList|ownerId|uploaderUid|StoredFile|forbidden|403|permission|role|authorize|hasAccess|assert.*Scenario|assert.*Group)" $(cat /tmp/vps_files.txt) || true

Repository: UoaWDCC/VPS

Length of output: 5413

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show top of backend/src/routes/api/files.js to capture middleware wiring/order
sed -n '1,120p' backend/src/routes/api/files.js | nl -ba | head -n 120

# Inspect firebase auth middleware to see what it guarantees (e.g., req.user fields)
sed -n '1,220p' backend/src/middleware/firebaseAuth.js | cat -n

# Locate StoredFile model to see available fields (scenarioId/groupId/uploaderUid/etc.)
fd -t f 'storedfile*|StoredFile*|file*model*' backend/src/db/models backend/src/db --max-depth 6 2>/dev/null > /tmp/storedfile_models.txt || true
cat /tmp/storedfile_models.txt || true
for f in $(cat /tmp/storedfile_models.txt 2>/dev/null); do echo "---- $f"; sed -n '1,260p' "$f" | cat -n; done

# Find any ownership/permission checks around StoredFile elsewhere
rg -n --hidden "uploaderUid|ownerUid|scenarioId|groupId|access|permission|role|authorize|hasAccess|403|forbidden" backend/src/routes/api backend/src/middleware backend/src/db/daos backend/src/db/models | rg -n "StoredFile|gridFsId|files.js|/files|state-conditionals" || true

# Check if other handlers in this same files.js enforce scenario/group membership
rg -n --hidden "assertGroupInScenario|scenarioId|groupId" backend/src/routes/api/files.js

Repository: UoaWDCC/VPS

Length of output: 98

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Show full middleware wiring at top of files.js (no nl; use cat -n)
sed -n '1,120p' backend/src/routes/api/files.js | cat -n

# 2) Inspect firebase auth middleware
sed -n '1,260p' backend/src/middleware/firebaseAuth.js | cat -n

# 3) Locate and inspect StoredFile model (search by filename first)
fd -t f 'storedfile*|StoredFile*' backend/src/db/models backend/src/db | head -n 50

# If we found candidates, print them; otherwise rg for "name: " fields in StoredFile schema
for f in $(fd -t f 'storedfile*|StoredFile*' backend/src/db/models backend/src/db 2>/dev/null | head -n 20); do
  echo "---- $f"
  sed -n '1,260p' "$f" | cat -n
done

# 4) Find any scenario/role/ownership checks referenced from files.js (and any middleware that looks like it)
rg -n --hidden "(scenarioAuth|dashboardAuth|permission|role|ownerUid|uploaderUid|accessList|hasAccess|authorize|forbidden|403|assert.*Scenario|assert.*Group)" backend/src/routes/api/files.js backend/src/routes/api backend/src/middleware backend/src/db/daos backend/src/db/models || true

# 5) Confirm how this router is mounted (to see if additional middleware wraps it)
rg -n --hidden "routes/api/files|api/files|require\\(.*files|from '.*files|import .*files\\.js" backend/src | head -n 100 || true

Repository: UoaWDCC/VPS

Length of output: 20512

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect backend/src/index.js around where files router is mounted
sed -n '1,120p' backend/src/index.js | cat -n
rg -n "app\.use\(\"/api/files\"|filesRouter" backend/src/index.js

# Inspect remaining part of backend/src/routes/api/files.js for any ownership checks in other file mutations
rg -n "router\.(patch|delete|post|put)\\(\"/:fileId\"|state-conditionals|scenarioAuth|dashboardAuth|assertGroupInScenario|accessList|getAccessList|403|forbidden" backend/src/routes/api/files.js
sed -n '140,280p' backend/src/routes/api/files.js | cat -n

# Inspect delete/state-conditionals handlers quickly for auth/ownership checks usage
sed -n '170,280p' backend/src/routes/api/files.js | cat -n

Repository: UoaWDCC/VPS

Length of output: 10731

Add authorization for file renames (and other file mutations) to prevent IDOR

backend/src/routes/api/files.js only applies router.use(auth) (Firebase token validation) and then PATCH /:fileId does StoredFile.findById(fileId) + updates meta.name without checking the caller’s access to the file’s scenarioId/groupId (same issue in DELETE /:fileId and the state-conditionals routes). Any authenticated user can rename/delete/modify arbitrary files by guessing fileId. Enforce scenario/group access (e.g., via the existing dashboardAuth/scenarioAuth/access checks) before mutating StoredFile.

🤖 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 `@backend/src/routes/api/files.js` around lines 152 - 172, The PATCH handler is
missing an authorization check and allows IDOR; after fetching meta via
StoredFile.findById(fileId) (and similarly in DELETE /:fileId and
state-conditionals handlers), verify the caller has access to the file's
scenarioId/groupId using the existing authorization helpers (e.g., call
scenarioAuth or dashboardAuth or the same access-check logic used elsewhere)
using the authenticated principal from req (uid or user on req set by
router.use(auth)); if the check fails return 403 and do not modify/save the
StoredFile, otherwise proceed to set meta.name and save. Ensure the same pattern
is applied to delete and other mutation routes that touch StoredFile.


/**
* @route DELETE /api/files/:fileId
* @desc Delete a stored file and its GridFS data
Expand Down
170 changes: 143 additions & 27 deletions frontend/src/features/resources/ManageResourcesPage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
UsersIcon,
PlusIcon,
XIcon,
PencilIcon,
CheckIcon,
} from "lucide-react";
import AddGroup from "./components/AddGroup";
import StateConditionalMenu from "../../components/StateVariables/StateConditionalMenu";
Expand Down Expand Up @@ -47,6 +49,8 @@ export default function ManageResourcesPage() {
// Groups (each with files)
const [groups, setGroups] = useState([]);
const [selectedFile, setSelectedFile] = useState(null);
const [renamingFileId, setRenamingFileId] = useState(null);
const [renameInput, setRenameInput] = useState("");

// Load groups and files
useEffect(() => {
Expand Down Expand Up @@ -133,6 +137,55 @@ export default function ManageResourcesPage() {
}
}

async function renameFile(fileId, newName) {
try {
const user = getAuth().currentUser;
if (!user) {
toast.error("You must be logged in.");
return;
}
const idToken = await user.getIdToken();
const { data } = await axios.patch(
`/api/files/${fileId}`,
{ name: newName },
{ headers: { Authorization: `Bearer ${idToken}` } }
);

setGroups((prev) =>
prev.map((g) => ({
...g,
files: (g.files || []).map((f) =>
f.id === fileId ? { ...f, name: data.name } : f
),
}))
);

if (selectedFile?.id === fileId) {
setSelectedFile((prev) => ({ ...prev, name: data.name }));
}

toast.success("Renamed");
} catch (err) {
console.error(err);
toast.error(err?.response?.data?.error || "Rename failed");
}
}

async function handleRenameSubmit() {
const trimmed = renameInput.trim();
if (!trimmed) {
setRenamingFileId(null);
return;
}
const current = groups
.flatMap((g) => g.files)
.find((f) => f.id === renamingFileId);
if (current && trimmed !== current.name) {
await renameFile(renamingFileId, trimmed);
}
setRenamingFileId(null);
}

async function removeFile(fileId) {
try {
const user = getAuth().currentUser;
Expand Down Expand Up @@ -234,7 +287,7 @@ export default function ManageResourcesPage() {

<div className="grid grid-cols-1 lg:grid-cols-3 gap-4">
{/* LEFT: Groups and files */}
<div className="card bg-base-100 shadow-md">
<div className="card bg-base-100 shadow-md overflow-hidden min-w-0">
<div className="card-body gap-4 px-0">
<div className="flex items-center justify-between gap-2">
<h2 className="text-m">Collections</h2>
Expand Down Expand Up @@ -271,7 +324,7 @@ export default function ManageResourcesPage() {
<ul className="menu bg-base-100 rounded-box w-full">
{groups.map((group) => (
<li key={group.id}>
<details>
<details style={{ overflow: "hidden" }}>
<summary className="flex items-center">
<span className="text--1 truncate">
{group.name}
Expand All @@ -294,34 +347,93 @@ export default function ManageResourcesPage() {
</div>
</summary>

<ul>
<ul className="overflow-hidden">
{group.files.length === 0 && (
<li className="opacity-60 p-2">No files yet</li>
)}

{group.files.map((f) => (
<li key={f.id}>
<div className="flex items-center justify-between">
<a
className="min-w-0 flex-1 text--1 truncate"
onClick={() =>
setSelectedFile({
...f,
groupId: group.id,
groupName: group.name,
})
}
>
{f.name}
</a>
<button
className="btn btn-phantom btn-xs px-0"
onClick={() => removeFile(f.id)}
title="Delete file"
<li key={f.id} className="overflow-hidden">
{renamingFileId === f.id ? (
<div className="flex items-center gap-1 py-1">
<input
autoFocus
className="input input-bordered input-xs flex-1 min-w-0"
value={renameInput}
maxLength={255}
onChange={(e) =>
setRenameInput(e.target.value)
}
onKeyDown={(e) => {
if (e.key === "Enter")
handleRenameSubmit();
if (e.key === "Escape")
setRenamingFileId(null);
}}
/>
<button
className="btn btn-primary btn-xs"
onClick={handleRenameSubmit}
title="Save"
>
<CheckIcon size={14} />
</button>
<button
className="btn btn-phantom btn-xs px-0"
onClick={() => setRenamingFileId(null)}
title="Cancel"
>
<XIcon size={14} />
</button>
</div>
) : (
<div
style={{
display: "grid",
gridTemplateColumns:
"minmax(0, 1fr) auto auto",
alignItems: "center",
gap: "4px",
overflow: "hidden",
}}
>
<XIcon size={16} />
</button>
</div>
<a
className="text--1"
title={f.name}
style={{
overflow: "hidden",
textOverflow: "ellipsis",
whiteSpace: "nowrap",
}}
onClick={() =>
setSelectedFile({
...f,
groupId: group.id,
groupName: group.name,
})
}
>
{f.name}
</a>
<button
className="btn btn-phantom btn-xs px-0"
onClick={() => {
setRenameInput(f.name);
setRenamingFileId(f.id);
}}
title="Rename file"
>
<PencilIcon size={14} />
</button>
<button
className="btn btn-phantom btn-xs px-0"
onClick={() => removeFile(f.id)}
title="Delete file"
>
<XIcon size={16} />
</button>
</div>
)}
</li>
))}
</ul>
Expand Down Expand Up @@ -462,10 +574,14 @@ function Preview({ file }) {

return (
<div className="space-y-3">
<div className="flex items-center justify-between">
<h3 className="text-m">{file.name}</h3>
<div className="flex items-start justify-between gap-2">
<h3 className="text-m break-all">{file.name}</h3>
{downloadUrl && (
<a className="btn btn-phantom btn-xs" href={downloadUrl} download>
<a
className="btn btn-phantom btn-xs flex-shrink-0"
href={downloadUrl}
Comment on lines +580 to +582
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Tailwind CSS v4.0.7, is flex-shrink-0still supported, or shouldshrink-0 be used instead?

💡 Result:

In Tailwind CSS v4.0.7, you should use shrink-0 instead of flex-shrink-0. While flex-shrink-0 was previously deprecated and initially removed in the early stages of Tailwind CSS v4 [1], it was subsequently brought back as a legacy utility to maintain backward compatibility [2]. However, shrink-0 is the modern, preferred utility class [3][4]. You are encouraged to use shrink-0 in new code, as flex-shrink-0 is considered a legacy alias [1][2].

Citations:

🌐 Web query:

Tailwind CSS v4 flex-shrink-0alias forshrink-0 documentation

💡 Result:

In Tailwind CSS v4, the documentation for the flex-shrink utilities uses shrink-* as the canonical class names. Specifically, v4 documents the utility as shrink-0 (not flex-shrink-0) to set flex-shrink: 0 [1]. flex-shrink-0 is a v3 legacy class name that maps to the v4 alternative shrink-0 during the v4 migration/upgrade process [2][3].

Citations:

Use Tailwind v4 shrink-0 instead of flex-shrink-0

In frontend/src/features/resources/ManageResourcesPage.jsx (line ~581), change flex-shrink-0 to shrink-0 since Tailwind v4 documents shrink-* as the canonical utility for flex-shrink.

🤖 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 `@frontend/src/features/resources/ManageResourcesPage.jsx` around lines 580 -
582, In the ManageResourcesPage component, update the anchor element that
renders the download link (the <a> with href={downloadUrl}) to replace the
deprecated Tailwind utility class "flex-shrink-0" with the v4 canonical
"shrink-0" in its className string so the download button uses the correct
shrinking behavior.

download
>
Download
</a>
)}
Expand Down
Loading