fix(publicshare): [OCISDEV-861] bound the ListGrants stat fan-out

[LukasHirt]

Summary

Fixes OCISDEV-861: ListPublicShares issued one gateway Stat RPC per public link not created by the calling user, exceeding the request deadline on tenants with many links. The permission check is unchanged — every user sees exactly the same links as before — but the cost of performing it is now bounded.

Acceptance-test counterpart: owncloud/ocis#12783 (merged; APITESTS_COMMITID bumped to 00aeefac3b23efbb4b7fe67225e74a8a6e6b71f9).

The bug

pkg/publicshare/manager/json/json.go's ListPublicShares iterated the entire tenant public-link DB and, for every foreign link, issued a synchronous gateway Stat purely to read PermissionSet.ListGrants — discarding the rest of the response.

Observed in production on a tenant with ~3000 links: ~3000 serial RPCs in one request, deadline exceeded, 3091x listshares: an error occurred during stat on the resource with code = Canceled, infinite spinner in the Web "Shares" page. Independently corroborated by OCISDEV-1196: protojson.Unmarshal on this path accounted for 5.4% of storage-process CPU across 12,937 calls in 5 minutes.

This is the production default path — init() registers the driver name jsoncs3 onto this same type, and ocis defaults to jsoncs3. There is no separate manager/jsoncs3 package.

The approach

The Stat check is kept, not replaced. Four earlier attempts tried to substitute a cheaper precomputed lookup (space membership, +grant spaces, ListReceivedShares, a tenant-wide admin capability check). All of them changed who could see what, and two introduced a privilege escalation. The reason is structural: ListGrants on a resource is the OR of every ACE from that resource up to the space root (assemblePermissions, which also short-circuits on deny grants), so it cannot be derived from any set of space or resource ids. Keeping the original check is what makes visibility parity provable rather than argued.

ListPublicShares is now two passes:

1. In-memory, no RPCs. Walk the DB once. Expiry, filters and the nil-resource_id guard behave exactly as before. Split survivors into own shares (no permission check needed) and foreign shares, collecting the set of distinct resource ids the foreign ones point at.
2. Bounded RPCs. Stat each distinct resource at most once, through a worker pool of at most 5, then filter in memory.

Three consequences:

• Per-resource dedup, both directions. The old cache recorded only allowed resources, so a denied resource was re-stat'd once per link pointing at it — the amplifier on the PROPFIND path, which passes one filter per directory child. Now allowed and denied answers are both memoised.
• Bounded concurrency instead of one RPC after another.
• No RPCs at all when the caller created every link.

Cache keys use storagespace.FormatResourceID (the old key omitted SpaceId while utils.ResourceIDEqual compares it — a cross-space collision), and the Stat carries FieldMask{Paths: ["permissions"]} so the gateway stops marshalling full resource info to read one bool.

Deadline handling

If the caller's context has a deadline, the stat phase stops shortly before it expires and returns a partial list with a single warning, rather than the whole request dying with code = Canceled. If the caller sets no deadline, no bound is imposed.

Deliberately no new configuration: this is a mitigation until an indexed public-share backend replaces this manager, and there is no value in teaching operators knobs that will be removed again. Concurrency is an internal constant; the deadline is derived from the caller and never capped by an invented value.

Measurements

Real oCIS in Docker — two images built from source (unpatched vs this branch), 10,000 real public links created through the API by one user, listed by a different user who is a space Manager. Median of 3:

Links	Unpatched	This branch
100	140 ms	97 ms
1,000	996 ms	604 ms
3,000	2,783 ms	1,702 ms
10,000	9,074 ms	5,753 ms

At 10,000 links the unpatched build took 15.5 s on a cold cache and ~9.8 s warm — straddling a 10 s client deadline, and failing outright under curl --max-time 10 on first call. This branch: 6.45 s cold, ~5.7 s warm, comfortably under.

Every cell returned byte-identical response bodies between the two builds (16,346,826 bytes at 10,000 links) — the strongest available evidence that no share is lost.

Two caveats stated plainly. First, that fixture puts every link on its own file, which is the worst case for per-resource dedup — so this 1.4–1.6× reflects bounded concurrency alone. A synthetic run where 3,000 links sat on 50 shared folders showed the dedup and negative-caching paths reducing 2,999 stats to 50 (~109×), which is closer to how a real tenant looks but has not been measured against live oCIS. Second, loopback gRPC costs ~1 ms per Stat versus tens of ms in production, so absolute timings here understate the real-world gap.

Testing

15 specs, green under -race:

• The regression test drives the bug directly: a 501-row fixture asserting zero Stat calls for own shares, written first and failing with a measured 500 stats.
• Dedup: 500 links over 5 distinct resources ⇒ exactly 5 stats.
• Negative caching: two links on the same denied resource ⇒ exactly 1 stat.
• Bounded concurrency: asserts in-flight stats never exceed the bound and actually reach it, so it fails if concurrency breaks or silently goes serial.
• Deadline: fails closed (a resource never decided is never included) and returns a partial list without an error; a generous caller deadline is honoured in full, so no internal ceiling can creep back.
• Nil-resource_id guards for both filter types.

Several are mutation-verified — deleting the ListGrants check, keying on SpaceId alone, or dropping negative caching each fail exactly the intended spec and nothing else.

Also fixed here

A nil-pointer panic in pkg/publicshare/publicshare.go: MatchesFilter's StorageIDFilterType branch dereferenced share.ResourceId.StorageId unguarded. A persisted row with nil resource_id (real — OCISDEV-877 shipped a repair CLI for it) panicked, reachable because MatchesFilters runs before the manager's own nil guard while the gateway's space-purge passes a StorageIDFilter. Pre-existing; fixed here because it sits inside the code path under change, and regression-tested.

[kw-security]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[LarsJurgensen]

Why do we check cache here? don't we deduplicate the resources already early in memory? And we initialize the cache from scratch before, so it's not kept between user requests, correct? So is a cache hit even possible here?

[LarsJurgensen]

If the above is correct, I'd also rename it from cache to something like resultMap. Since it wouldn't really be caching, just collecting the result, no?