Skip to content

fix(cf-edit): keep Hub ACL authorization outside Branch(master) - #44

Merged
opendev9 merged 2 commits into
devfrom
fix/cf-edit-publish-acl-lock
Oct 25, 2026
Merged

opendev9 merged 2 commits into
devfrom
fix/cf-edit-publish-acl-lock

Conversation

@opendev9

@opendev9 opendev9 commented Oct 25, 2026 •

Copy link
Copy Markdown

What and why

Fixes #43. cf-edit publication (save / commit / checkin, including the
optimistic action available with Checkout OFF) used to do this:

  1. BeginTx
  2. cfEditHead → SELECT commit_id FROM Branch WHERE ... FOR UPDATE
  3. guard row checks
  4. cfEditCurrentPermission → cfACLAuthorize → HTTP POST Hub
    /cloudfile/acl/check/ with a hard 5s client timeout
  5. create commit, update Branch, Commit

So every publication of an ACL-protected library serialized on Branch(master)
behind a remote ACL round trip — the same defect class #42 fixed for original
commit / Branch CAS, but #42 did not touch cf_v2_edit_agent.go.

Change

Two-phase publication, reusing the contract #42 established (no new mechanism,
table, queue or second policy engine):

  • Phase A (before BeginTx): cfEditPermissionPreflight reads the bounded
    cf_dir_acl row/revision proof (cfACLCurrentProof(..., locked=false)) and,
    only for a protected library, makes one Hub write decision for the exact path
    (cfACLAuthorize; the handler keeps its existing 403 mapping).
  • Phase B (Branch already locked): cfACLFinal re-reads the same proof with
    current locking reads and compares the SHA-256 digest. No Hub HTTP, no commit
    diff, no object-store traversal under the lock.
  • HEAD/Guard semantics unchanged (VERSION_CHANGED, STALE_GUARD,
    CHECKED_OUT). A rule change between phases refuses the publication
    (PERMISSION_CHANGED), matching perf(native): move C/Go ACL HTTP outside Branch transactions #42's fail-closed behaviour; reviewed Hub ACL
    writers take the same Branch(master) lock, so the proof is the
    linearization point.
  • cfEditCurrentPermission removed (its only caller was this path).
  • Follow-up from perf(native): cf-edit publish still calls Hub ACL check while holding Branch(master) FOR UPDATE #43: removed the now-callerless C wrapper
    cf_v2_acl_final_scoped() and its header declaration. Keeping it invited the
    forbidden "Hub HTTP inside a caller transaction" use.

Files: fileserver/cf_v2_edit_agent.go, common/cf-v2-acl.c,
common/cf-v2-acl.h, plus tests/test_cf_edit_publish_lock_scope.py (cheap
source-order contract that fails if a Hub authorization returns to the
publication transaction).

Verification

Local (PR head 143678c):

  • go build ./... / go vet ./... — OK
  • go test . -run 'TestCFEdit|TestCFACL|TestCFV2|TestUploadGuard' -count=1 — ok
  • python3 -m unittest discover -s tests -p 'test_cf_*.py' — 21 tests OK
  • gofmt -l clean; git diff --check clean
  • GitHub quick source checks (PR gate) — SUCCESS

Manual CloudFile checks dispatch (run 40171818858, C build + cross-repo):

  • CC cf-v2-acl.o compiled successfully after the wrapper removal.
  • The job then fails at runtime with CloudFile V2 requires ENABLE_GO_FILESERVER=true and the checks job fails on
    cloudfile-docker/tools/check-requirements.txt (path moved). A dispatch of
    the same workflow on plain dev (run 40172403047) fails with the exact
    same signatures, so both are pre-existing workflow/environment drift, not
    introduced here.

Not verified / follow-up

  • Real MariaDB + Hub concurrency proof (delayed /cloudfile/acl/check/, assert
    Branch lock wait excludes the Hub RTT) still needs an integration
    environment; tests/test_cf_v2_acl_native.py is a __main__ script and was
    not executed here.
  • Hub docstring drift is fixed separately in docs(acl): describe the two-phase native ACL check cloudfile-hub#91.

Refs #42, #43.

cf-edit publication (optimistic save, commit, checkin) acquired
Branch(master) FOR UPDATE and then called Hub /cloudfile/acl/check/ inside
the same transaction through a client with a hard 5s timeout. A protected
library therefore serialized every publication behind a remote ACL round
trip, and a slow Hub blocked unrelated commits on the same repository.

Run the bounded ACL row/revision proof and the single Hub write decision as
a preflight before BeginTx, then verify the same proof with cfACLFinal()
under the lock. Phase B performs no HTTP and no commit diff. Reviewed Hub
ACL writers take the same Branch lock, so an ACL change between preflight
and publication refuses the write instead of being bypassed.

Verified locally: go build ./..., go vet ./..., targeted Go tests
(TestCFEdit/TestCFACL/TestCFV2/TestUploadGuard) and the cheap Python source
contract suite (21 tests) pass. Real MariaDB+Hub concurrency proof is still
out of scope here and tracked in #43.

Refs #43
cf_v2_acl_final_scoped() has no callers since #42 moved native CAS
publication to the preflight/proof split. Keeping it is a trap: it runs
cf_v2_acl_preflight_scoped() (including the Hub HTTP call) inside a caller
transaction, which is exactly the lock/HTTP ordering this branch fixes.
Delete the wrapper and its header declaration so the two-phase contract has
a single entry point. No behaviour change for existing callers.

Refs #43
@opendev9
opendev9 force-pushed the fix/cf-edit-publish-acl-lock branch from f7e490c to 143678c Compare October 25, 2026 07:10
@opendev9

Copy link
Copy Markdown
Author

CI 复核补充(C 编译与失败归因)

  • 手动触发 CloudFile checks(run 40171818858):
    • CC cf-v2-acl.o 编译通过(死包装删除没有 C 编译问题),随后进入运行阶段;
    • 运行阶段以 seafile-session.c(193): CloudFile V2 requires ENABLE_GO_FILESERVER=true 失败,checks job 以 cloudfile-docker/tools/check-requirements.txt 路径不存在失败。
  • 为排除本 PR 因素,对同一 workflow 在 plain dev 上再触发一次(run 40172403047):build-c 与 checks 的失败签名与上一轮完全一致。
  • 结论:两个失败均为上游 workflow/环境漂移(docker 仓 requirements 路径迁移、测试启动参数缺失),与本 PR 改动无关;本 PR 的 PR 门禁(quick source checks)为 SUCCESS。

@opendev9
opendev9 merged commit eebd3a6 into dev Oct 25, 2026
1 of 3 checks passed
@servicememeai
servicememeai deleted the fix/cf-edit-publish-acl-lock branch October 26, 2026 00:35
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.

perf(native): cf-edit publish still calls Hub ACL check while holding Branch(master) FOR UPDATE

1 participant