Skip to content

fix: resolve generated-output downloads per user - #800

Open
mvanhorn wants to merge 1 commit into
HKUDS:mainfrom
mvanhorn:fix/790-multi-user-output-download
Open

fix: resolve generated-output downloads per user#800
mvanhorn wants to merge 1 commit into
HKUDS:mainfrom
mvanhorn:fix/790-multi-user-output-download

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Description

Update SafeOutputStaticFiles in deeptutor/api/main.py to select a PathService for each request instead of delegating lookup to the startup-bound StaticFiles.directory: read the dt_token cookie from the ASGI request, decode it through the existing auth service, convert the payload through the existing multi-user identity mapping, and resolve that scope with get_path_service_for_scope. Resolve and serve the requested file against that service's public-output root only after is_public_output_path accepts it, so validation and physical lookup use the same account root. Preserve the default service only for auth-disabled/local-admin operation; when authentication is enabled, a missing or invalid credential should return the same non-revealing 404 rather than falling back to another account's files.

SafeOutputStaticFiles is constructed at application startup with the default PathService, so its Starlette directory remains rooted at the admin output tree for every request. Generated files for authenticated non-admin users are written beneath data/users/<uid>/user, causing otherwise valid /api/outputs/... links to return 404 even though the artifact exists. The bundle provides a concrete reproduction and diagnosis, and the current code still contains the startup-bound mount; there are no competing or prior cross-referenced PRs. The fix must preserve the existing public-output whitelist, private-suffix rejection, traversal protection, and account isolation.

Closes #790

Related Issues

  • Closes #...
  • Related to #...

Module(s) Affected

  • agents
    Not claimed: the workspace test run did not pass; see the notes above.
  • api
    Not claimed: the workspace test run did not pass; see the notes above.
  • config
    Not claimed: the workspace test run did not pass; see the notes above.
  • core
    Not claimed: the workspace test run did not pass; see the notes above.
  • knowledge
    Not claimed: the workspace test run did not pass; see the notes above.
  • logging
    Not claimed: the workspace test run did not pass; see the notes above.
  • services
    Not claimed: the workspace test run did not pass; see the notes above.
  • tools
    Not claimed: the workspace test run did not pass; see the notes above.
  • utils
    Not claimed: the workspace test run did not pass; see the notes above.
  • web (Frontend)
    Not claimed: the workspace test run did not pass; see the notes above.
  • docs (Documentation)
    Not claimed: the workspace test run did not pass; see the notes above.
  • scripts
    Not claimed: the workspace test run did not pass; see the notes above.
  • tests
    Not claimed: the workspace test run did not pass; see the notes above.
  • Other: ...
    Not claimed: the workspace test run did not pass; see the notes above.

Checklist

  • I have read and followed the contribution guidelines.
  • My code follows the project's coding standards.
  • I have run pre-commit run --all-files and fixed any issues.
    Not claimed: the workspace test run did not pass; see the notes above.
  • I have added relevant tests for my changes.
  • My changes do not introduce any new security vulnerabilities.

Additional Notes

Add any other context or screenshots about the pull request here.

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Looks like this landed upstream independently. 712d413 added deeptutor/api/routers/outputs.py, which serves outputs through a request-scoped path service and fails closed with a 404 when there is no authenticated user, and tests/api/test_output_files.py covers #790 directly.

I compared the two before saying anything: resolve_public_output_path already handles what my tests exercised, traversal outside the outputs root, private suffixes, and non-files. Its docstring also makes a fair point about the shape I used, which validated against one workspace and then rebuilt the path from another root.

Happy to close this out. Let me know if there is anything here worth keeping instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]:Multi-user File Download Fails with 404 "Output not found" on /api/outputs

1 participant