Skip to content

Share subfolder feature implemented#728

Draft
opencode-agent[bot] wants to merge 58 commits into
mainfrom
opencode/issue184-20260630050929
Draft

Share subfolder feature implemented#728
opencode-agent[bot] wants to merge 58 commits into
mainfrom
opencode/issue184-20260630050929

Conversation

@opencode-agent

Copy link
Copy Markdown
Contributor

Here's a summary of the implementation for the "Allow Share Subfolders" feature:

Backend Changes

DTO layer (backend/src/dto/shared_resource.go)

  • Added Subfolder string field with json:"subfolder,omitempty" tag to SharedResource

DBOM layer (backend/src/dbom/exported_share.go)

  • Added Subfolder string field with gorm:"default:''" tag to ExportedShare

Config layer (backend/src/config/addon_json_config.go)

  • Added Subfolder string field to config.Share struct

Converters (generated code)

  • Updated ExportedShareToShare, ExportedShareToSharedResource, SharedResourceToExportedShareNoUsersNoMountPointPath, ShareToExportedShareNoMountPointPath, and sharedResourceToExportedShare to map the Subfolder field

Service layer (backend/src/service/share_service.go)

  • Added validateSubfolder() — rejects absolute paths and .. traversal
  • Added ensureSubfolder() — creates the subfolder directory on disk with 0o755 permissions
  • Added validation call in CreateShare() and UpdateShare()
  • Added directory creation in CreateShare()

Server process service (backend/src/service/server_process_service.go)

  • jSONFromDatabase() resolves subfolder into config.Share.Path before template rendering, so the Samba template automatically uses the correct resolved path

Frontend Changes

ShareEditForm.tsx

  • Added subfolder to default form values
  • Added TextFieldElement "Subfolder (optional)" input with helper text, shown for non-internal shares

ShareDetailsPanel.tsx

  • Displays subfolder in the share config section
  • Shows subfolder appended to path in the mount point section

sratApi.ts (generated types)

  • Manually added subfolder?: string to SharedResource and SharedResourcePostData types (will be auto-regenerated on next bun run gen)

Tasks remaining (per task file):

  • Task 6, 7, 8: Tests (unit, API handler, frontend component)
  • Task 9: Regenerate frontend types with bun run gen
  • Task 10: Documentation updates

Closes #184

New%20session%20-%202026-06-30T05%3A09%3A28.260Z
opencode session  |  github run

Co-authored-by: dianlight <dianlight@users.noreply.github.com>
@opencode-agent

Copy link
Copy Markdown
Contributor Author

User opencode-agent[bot] does not have write permissions

github run

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.51282% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.22%. Comparing base (06f1527) to head (091f8dc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
backend/src/service/share_service.go 13.04% 17 Missing and 3 partials ⚠️
backend/src/converter/config_to_dbom_conv_gen.go 0.00% 4 Missing ⚠️
backend/src/service/server_process_service.go 25.00% 2 Missing and 1 partial ⚠️
backend/src/converter/dto_to_dbom_conv_gen.go 50.00% 1 Missing and 1 partial ⚠️
.../src/pages/shares/components/ShareDetailsPanel.tsx 0.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #728      +/-   ##
==========================================
- Coverage   43.24%   43.22%   -0.03%     
==========================================
  Files         297      297              
  Lines       29749    29786      +37     
  Branches     1932     1936       +4     
==========================================
+ Hits        12865    12874       +9     
- Misses      14950    14968      +18     
- Partials     1934     1944      +10     
Flag Coverage Δ
backend 39.12% <17.14%> (-0.03%) ⬇️
custom-component 43.22% <20.51%> (-0.03%) ⬇️
frontend 55.90% <50.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
backend/src/config/addon_json_config.go 50.00% <ø> (ø)
...tend/src/pages/shares/components/ShareEditForm.tsx 43.60% <100.00%> (+0.66%) ⬆️
backend/src/converter/dto_to_dbom_conv_gen.go 44.78% <50.00%> (+0.04%) ⬆️
.../src/pages/shares/components/ShareDetailsPanel.tsx 57.89% <0.00%> (-2.11%) ⬇️
backend/src/service/server_process_service.go 34.43% <25.00%> (-0.16%) ⬇️
backend/src/converter/config_to_dbom_conv_gen.go 0.00% <0.00%> (ø)
backend/src/service/share_service.go 58.51% <13.04%> (-2.97%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dianlight

Copy link
Copy Markdown
Owner

/oc subfolder is a lab feature available only in lab mode.

@opencode-agent

Copy link
Copy Markdown
Contributor Author

Here's my code review:

Issues Found

1. 🐛 False positive in validateSubfolder() — rejects valid folder names starting with ..

backend/src/service/share_service.go:266strings.HasPrefix(cleaned, "..") falsely rejects legit names like ..hidden or ..mydir. filepath.Clean("..hidden")"..hidden", and strings.HasPrefix("..hidden", "..") is true.

Fix: Check for .. as a path component, not a string prefix:

cleaned := filepath.Clean(subfolder)
if cleaned == ".." || strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) {
    return errors.New("subfolder must not traverse above mount root")
}

2. 🐛 ensureSubfolder() not called in UpdateShare()

backend/src/service/share_service.go:346CreateShare calls ensureSubfolder (line 329) but UpdateShare does not. If a share is created without a subfolder and later updated to add one, the directory is never created on disk.

3. 🐛 NFS exports ignore subfolder

backend/src/service/server_process_service.go:867writeNFSConfig() uses share.MountPointData.Path directly without resolving share.Subfolder. NFS exports point to the mount root instead of the subfolder, inconsistent with Samba config behavior.

Fix: Add subfolder resolution here matching the pattern in jSONFromDatabase():

path := share.MountPointData.Path
if share.Subfolder != "" {
    path = filepath.Join(path, share.Subfolder)
}

4. 📝 Missing DB migration

No goose migration (00017_add_share_subfolder.go) added for the new subfolder column. GORM AutoMigrate will add it silently, but the project has a pattern of explicit goose migrations (see migrations/00016_migrate_smart_mode.go).

5. 📝 Style: validateSubfolder returns error instead of errors.E

Minor inconsistency with the rest of share_service.go which uses errors.E from gitlab.com/tozd/go/errors.

6. 📝 Tasks 6-10 not yet done

Noted and acknowledged in the PR description. Tests, frontend type regeneration, and docs are still pending.

Conclusion: Blocks 1, 2, and 3 should be addressed before merge. Block 4 is a consistency concern.

New%20session%20-%202026-06-30T18%3A48%3A44.479Z
opencode session  |  github run

@opencode-agent

Copy link
Copy Markdown
Contributor Author

User github-actions[bot] does not have write permissions

github run

@dianlight dianlight marked this pull request as draft July 2, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow share subfolders

1 participant