Skip to content

fix(strm): respect deployment umask for local directories - #2931

Open
StromKuo wants to merge 2 commits into
OpenListTeam:mainfrom
StromKuo:feat/strm-local-permissions
Open

fix(strm): respect deployment umask for local directories#2931
StromKuo wants to merge 2 commits into
OpenListTeam:mainfrom
StromKuo:feat/strm-local-permissions

Conversation

@StromKuo

@StromKuo StromKuo commented Aug 15, 2026

Copy link
Copy Markdown
  • make locally saved STRM directories respect the deployment umask
  • preserve existing directory permissions without automatic chmod
  • add permission handling tests

Summary / 摘要

OpenList currently creates local STRM directories with 0700 permissions. When Plex or another media server runs under a different UID, it cannot traverse these directories and read the generated STRM files.

This PR changes local STRM directory creation to respect the deployment's umask instead of using the hardcoded 0700 mode:

  • New directories are created with 0777 before umask is applied.
  • Existing directory permissions are preserved and are not automatically modified.
  • Existing directories with old permissions can be adjusted deployment-side with chmod/chown or ACLs if needed.

OpenList 目前使用 0700 权限创建本地 STRM 目录。当 Plex 或其他媒体服务器以不同 UID 运行时,它无法遍历这些目录并读取生成的 STRM 文件。

本次 PR 将本地 STRM 目录创建方式改为遵循部署环境的 umask,而不是使用硬编码的 0700

  • 新目录以 0777 创建,再由 umask 决定最终权限。
  • 保留已有目录权限,不自动修改。
  • 如需调整旧目录权限,可由部署侧一次性使用 chmod/chown 或 ACL 处理。
  • This PR has breaking changes.
    / 此 PR 包含破坏性变更。
  • This PR changes public API, config, storage format, or migration behavior.
    / 此 PR 修改了公开 API、配置、存储格式或迁移行为。
  • This PR requires corresponding changes in related repositories.
    / 此 PR 需要关联仓库同步修改。

Related repository PRs / 关联仓库 PR:

  • OpenList-Frontend: No functional frontend change required.
  • OpenList-Docs: None.

Testing / 测试

  • 尝试执行了 go test ./...,但其他驱动、internal/netpkg/aria2/rpc 中仍存在无关的既有失败。
  • Manual test / 手动测试: 未针对新构建的 OpenList 容器执行。

Checklist / 检查清单

  • I have read CONTRIBUTING.
    / 我已阅读 CONTRIBUTING
  • I confirm this contribution follows the repository license, contribution policy, and code of conduct.
    / 我确认这次贡献符合仓库许可证、贡献规范和行为准则。
  • I have formatted the changed code with gofmt, go fmt, or prettier where applicable.
    / 我已按适用情况使用 gofmtgo fmtprettier 格式化变更代码。
  • I have requested review from relevant maintainers or code owners where applicable.
    / 我已在适用情况下请求相关维护者审查。

AI Disclosure / AI 使用声明

  • This PR includes AI-assisted content.
    / 此 PR 包含 AI 辅助内容。

Tools used / 使用工具:

  • ChatGPT
  • Codex
  • GitHub Copilot
  • Claude
  • Gemini
  • Other (please specify) / 其他(请注明):

Usage scope / 使用范围:

  • Code generation / 代码生成

  • Refactoring / 重构

  • Documentation / 文档

  • Tests / 测试

  • Translation / 翻译

  • Review assistance / 审查辅助

  • I have reviewed and validated all AI-assisted content included in this PR.
    / 我已审核并验证所有包含在此 PR 中的 AI 辅助内容。

  • I have ensured that all AI-assisted commits include Co-Authored-By attribution.
    / 我已确保所有 AI 辅助提交都包含 Co-Authored-By 归属信息。

  • I can reproduce all AI-assisted content included in this PR without any AI tools.
    / 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。

- add private and shared permission modes for local STRM files
- repair shared-mode directory and file permissions during generation
- add permission handling tests

Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
@jyxjjj

jyxjjj commented Aug 15, 2026

Copy link
Copy Markdown
Member

Could you explain why this should be handled by an application-level private/shared permission mode instead of relying on the local umask/UID/GID configuration?

I understand that umask only affects newly created files and therefore cannot repair permissions on existing STRM files/directories. However, this PR also makes OpenList actively chmod the save path and existing files to fixed 0755/0644 modes.

What is the reason for preferring this over leaving filesystem permission policy to the deployment environment?

@jyxjjj

jyxjjj commented Aug 15, 2026

Copy link
Copy Markdown
Member

Just to clarify why I’m asking: my concern is mainly about minimizing the filesystem-related attack surface and keeping permission policy outside the application where possible.

I understand that explicitly managing permissions in the application is common, and that umask alone cannot repair permissions on existing files/directories.

@StromKuo

Copy link
Copy Markdown
Author

Could you explain why this should be handled by an application-level private/shared permission mode instead of relying on the local umask/UID/GID configuration?

I understand that umask only affects newly created files and therefore cannot repair permissions on existing STRM files/directories. However, this PR also makes OpenList actively chmod the save path and existing files to fixed 0755/0644 modes.

What is the reason for preferring this over leaving filesystem permission policy to the deployment environment?

Just to clarify why I’m asking: my concern is mainly about minimizing the filesystem-related attack surface and keeping permission policy outside the application where possible.

I understand that explicitly managing permissions in the application is common, and that umask alone cannot repair permissions on existing files/directories.

Thanks for clarifying. I share the goal of minimizing the filesystem-related attack surface and keeping permission policy in the deployment environment.

The motivation for this PR comes from a concrete issue in the current STRM implementation: local STRM directories are created through CreateNestedDirectory, which uses a hardcoded 0700 mode. This means that even when the deployment uses UMASK=022, newly created STRM directories are still not traversable by another service.

In my NAS deployment, OpenList and Plex share a bind-mounted directory but run under different UIDs without a common group. As a result, Plex cannot access newly generated STRM directories. STRM files themselves are created with the normal 0666 mode and already respect umask; the main issue is the hardcoded directory mode.

I agree that the current implementation is too broad because it forces 0755/0644 and chmods existing paths during normal updates. This could override deployment-specific ACL or permission policies.

I plan to revise the PR by removing the application-level private/shared permission mode and all automatic chmod operations. Instead, the STRM directory creation path will respect the deployment's umask rather than hardcoding 0700. Existing directories with old permissions will be handled through a one-time deployment-side chmod/chown or ACL adjustment.

Would this direction better fit the project's security and permission model?

jyxjjj commented Aug 15, 2026

Copy link
Copy Markdown
Member

Yes, that direction addresses my concern. Letting newly created STRM directories respect the deployment's umask, while leaving existing permissions to deployment-side migration, seems like a cleaner separation of responsibilities. Thanks for the clarification.

I'll take another look once the PR is updated.

- remove application-level permission modes and chmod operations
- create local STRM directories with umask-controlled permissions
- preserve existing permissions and test the behavior

Co-authored-by: Codex <267193182+codex@users.noreply.github.com>

@jyxjjj jyxjjj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The implementation now looks good to me. One remaining suggestion before merge: could you update the PR title and description to reflect the revised approach?

The current title/body still describe the removed private/shared permission mode, fixed 0755/0644 permissions, and permission repair for existing files/directories.

The final implementation instead makes newly created STRM directories respect the deployment umask, preserves existing permissions, and does not introduce a new permission-related config/API.

A title such as fix(strm): respect deployment umask for local directories would better match the current change.

@jyxjjj jyxjjj changed the title feat(strm): add local save permission mode fix(strm): respect deployment umask for local directories Aug 15, 2026

jyxjjj commented Aug 15, 2026

Copy link
Copy Markdown
Member

I’ve updated the PR title and description to match the revised implementation. I only adjusted the now-outdated permission-mode/chmod description and related API/config checkbox; the rest of the PR content was left unchanged.

@StromKuo

Copy link
Copy Markdown
Author

Thanks for updating the title and description. They now accurately reflect the revised implementation.

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.

2 participants