Details
### Summary
`GHSA-c8r8-95hg-mp34` added a centralized guard,
`util.IsForbiddenAbsPath()`, specifically to block access to a small
set of sensitive files: `conf/conf.json` (plaintext
`accessAuthCode`/API token/cookie key), `data/snippets/conf.json`,
the entire `data/templates/` directory, and
`data/.siyuan/publishAccess.json` (plaintext publish-mode passwords).
It was applied to `kernel/api/file.go` and `kernel/mcp/tools/file.go`.
Two other routes in the same server that serve arbitrary files by path,
`/history/*path` and `/repo/diff/*path`, construct their target paths
independently and were not updated to call this new guard. Since the
repo/history snapshot system's tracked root is `data/` (confirmed by
`getSyncIgnoreLines()`, whose ignore file lives at
`data/.siyuan/syncignore` with entries relative to `data/`), both
`data/.siyuan/publishAccess.json` and `data/templates/*` fall within
the scope that can legitimately be captured in historical snapshots,
meaning a prior version of either file can exist in
`util.HistoryDir`/the repo-diff temp checkout even after the live file
has been protected by the new guard. This is CWE-862 (Missing
Authorization) applied to a very recently introduced protection
mechanism.
### Details
`kernel/server/serve.go`, `/history/*path` (around line 994):
```go
ginServer.GET("/history/*path", model.CheckAuth, model.CheckAdminRole, func(context *gin.Context) {
p := filepath.Join(util.HistoryDir, context.Param("path"))
// 加密笔记本的历史是密文(.sy/assets/AV),需先解密再输出
if serveEncryptedHistory(context, p) {
return
}
secureAssetContentHeaders(context, p, p)
http.ServeFile(context.Writer, context.Request, p)
})
```
No call to `util.IsForbiddenAbsPath(p)` anywhere in this handler.
`kernel/server/serve.go`, `/repo/diff/*path` (around line 1241):
```go
ginServer.GET("/repo/diff/*path", model.CheckAuth, model.CheckAdminRole, func(context *gin.Context) {
requestPath := filepath.Clean(context.Param("path"))
if strings.Contains(requestPath, "..") {
context.Status(http.StatusUnauthorized)
return
}
...
p := filepath.Join(repoDiffBaseDir, requestPath)
if !gulu.File.IsSubPath(repoDiffBaseDir, p) {
context.Status(http.StatusUnauthorized)
return
}
http.ServeFile(context.Writer, context.Request, p)
})
```
This route does have its own traversal protection (`..` rejection and
`IsSubPath` containment within `repoDiffBaseDir`), but that only
prevents escaping the diff-checkout directory, it does nothing to
prevent retrieving a *legitimately checked-out historical copy* of
`publishAccess.json` or a templates file from within that directory,
which is exactly what the new guard exists to prevent regardless of
which directory the copy currently sits in.
`util.IsForbiddenAbsPath()` itself (`kernel/util/path_guard.go`,
introduced by the referenced fix) confirms the intended scope:
```go
// 禁止访问 data/.siyuan/publishAccess.json(含发布模式明文访问密码)
publishAccessPath := NormalizeAndResolve(filepath.Join(DataDir, ".siyuan", "publishAccess.json"))
if fileNorm == publishAccessPath {
return true
}
```
and
```go
// 禁止访问 data/templates 目录(含目录本身及其全部子路径)
templatesBase := NormalizeAndResolve(filepath.Join(DataDir, "templates"))
if fileNorm == templatesBase || gulu.File.IsSubPath(templatesBase, fileNorm) {
return true
}
```
Both are paths within `data/`, the same root the sync/history/repo
system tracks.
### Step-by-step reproduction
1. As the workspace admin, enable Publish with a password on at least
one notebook (creating `data/.siyuan/publishAccess.json` with a
plaintext password), then let a sync/backup snapshot capture this
state (or check whether local history capture already covers
`data/.siyuan/` in the deployed version).
2. Change or remove the publish password, so the live
`publishAccess.json` no longer contains the old plaintext password
the new guard is meant to hide, going forward.
3. As the admin, request the historical/diff version instead of the
live file:
```bash
curl -s http://<target>:6806/history/<snapshot-path-to-publishAccess.json> \
-u "<workspaceName>:<accessAuthCode>"
curl -s http://<target>:6806/repo/diff/<diff-path-to-publishAccess.json> \
-u "<workspaceName>:<accessAuthCode>"
```
4. Expected if consistently protected, matching the behavior the new
guard already provides on the live-file endpoints: rejected.
Observed: neither handler calls `IsForbiddenAbsPath`, so the
historical copy is served if it exists in that location.
*(Not run against a live compiled kernel, same sandbox limitation noted
throughout this review; both handlers are read directly from source at
the reviewed commit, and `IsForbiddenAbsPath`'s scope, plus the
sync-root confirmation via `getSyncIgnoreLines()`, are quoted directly
above. Whether these specific files are captured by history/repo
snapshots in a given deployment depends on the workspace's actual
usage history and was not independently verified against a live
instance in this review.)*
### Impact
An admin-authenticated request to either route can potentially retrieve
a historical copy of `data/.siyuan/publishAccess.json` (disclosing a
plaintext publish-mode password even after it has been changed or the
live file has been protected) or a `data/templates/*` file, directly
undermining the protection `GHSA-c8r8-95hg-mp34` was written four days
prior to this review specifically to provide, via two routes that
predate that fix and were not updated alongside it.
```
## Affected products
| Field | Value |
|---|---|
| Ecosystem | **Go** |
| Package name | `github.com/siyuan-note/siyuan/kernel` |
| Affected versions | Present as of commit `251596f` (2026-08-12, the version this review confirmed), i.e. postdates and was not covered by the `GHSA-c8r8-95hg-mp34` fix (commit `3542530`, 2026-08-08) |
| Patched versions | *(none yet, leave blank until a fix is released)* |
## Severity
| Field | Value |
|---|---|
| Vector string | `CVSS:3.1/AV:N/AC:L/PR:H/UI:N/S:U/C:M/I:N/A:N` |
| Score | **~5.9 (Medium)**, `PR:H` since admin authentication is required at the HTTP layer, confidentiality impact scoped to whatever sensitive historical content happens to exist in the tracked snapshots for a given deployment (a real but deployment-dependent condition, honestly reflected as Medium rather than assumed to always be present), no integrity/availability impact since both are read-only. |
## Weaknesses (CWE)
- **CWE-862**: Missing Authorization (primary)
- **CWE-200**: Exposure of Sensitive Information to an Unauthorized Actor
## Notes for filing
- Direct, narrow follow-up to `GHSA-c8r8-95hg-mp34`; recommend
referencing that advisory directly when filing, since this is
precisely the "sibling caller missed" pattern that fix's own
centralization (moving the check into a shared `util` function) was
presumably intended to prevent, just for two callers that existed
before the shared function did and weren't migrated to it.
- Suggested fix: add `if util.IsForbiddenAbsPath(p) { ... reject ... }`
to both handlers, matching the pattern already applied in
`kernel/api/file.go` and `kernel/mcp/tools/file.go`.