Skip to content

fix: knowledge document strategy permission #7745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
peslyn merged 1 commit into from
Nov 9, 2026
Merged

fix: knowledge document strategy permission #7745

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ui/src/views/knowledge-detail/setting/document-strategy/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { useKnowledgeDetailContext } from '../../context'
/**
* 知识库「文档处理策略」权限判断
*/
type KnowledgeExternalServicePermissionMethod = 'knowledgeExternalServiceRead' | 'knowledgeExternalServiceEdit'
type KnowledgeDocumentStrategyPermissionMethod = 'documentStrategyRead' | 'documentStrategyEdit'

function knowledgeExternalServicePermission(method: KnowledgeExternalServicePermissionMethod): boolean {
function knowledgeDocumentStrategyPermission(method: KnowledgeDocumentStrategyPermissionMethod): boolean {
if (isSystemResource()) return Boolean(perm.knowledge.system[method]())
if (isSystemSharedResource()) return Boolean(perm.knowledge.share[method]())
if (isWorkspaceSharedResource()) return false // 共享知识库在工作空间侧只读
Expand All @@ -38,7 +38,7 @@ watch(
/* 只保存文档处理策略，不触发同步或修改其他设置。 */
function handleSave() {
const knowledgeApi = resourceApi.knowledge
if (!knowledgeExternalServicePermission('knowledgeExternalServiceEdit') || !('putKnowledge' in knowledgeApi)) return
if (!knowledgeDocumentStrategyPermission('documentStrategyEdit') || !('putKnowledge' in knowledgeApi)) return
const detail = knowledge.value
const strategyForm = strategyFormRef.value
if (saving.value || !detail || !strategyForm) return
Expand Down Expand Up @@ -73,6 +73,6 @@ defineExpose({ customHeader: true })
<div v-loading="saving" class="max-w-200">
<DocumentStrategyForm ref="strategyFormRef" />
<!-- 保存文档处理策略 -->
<el-button v-if="knowledgeExternalServicePermission('knowledgeExternalServiceEdit')" type="primary" class="mt-4" :loading="saving" @click="handleSave">保存</el-button>
<el-button v-if="knowledgeDocumentStrategyPermission('documentStrategyEdit')" type="primary" class="mt-4" :loading="saving" @click="handleSave">保存</el-button>
</div>
</template>

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The diff is syntactically consistent, but there are a few review points:

  1. Likely permission bug for workspace-shared resources

    if (isWorkspaceSharedResource()) return false // 共享知识库在工作空间侧只读

    This returns false for both documentStrategyRead and documentStrategyEdit. If the function is ever called with documentStrategyRead, read access is denied, which contradicts the “read-only” comment.
    Suggested fix if read should be allowed:

    if (isWorkspaceSharedResource()) {
  return method === 'documentStrategyRead'
}

    Or, if only edit is ever used, narrow the type/function to edit-only to avoid misleading semantics.

  2. Permission key rename is a breaking change
    The permission methods changed from:

    • knowledgeExternalServiceRead → documentStrategyRead
    • knowledgeExternalServiceEdit → documentStrategyEdit

    Ensure perm.knowledge.system and perm.knowledge.share actually expose these new method names. Otherwise:

    perm.knowledge.system[method]()

    can throw if [method] is undefined. Also check backend permission codes, role configuration, tests, audit logs, and any persisted permissions for migration/backward compatibility.

  3. Runtime safety for permission lookup
    Consider guarding the dynamic call:

    return Boolean(perm.knowledge?.system?.[method]?.())

    and similarly for perm.knowledge.share. This avoids crashes if permissions are not loaded yet or a key is missing.

  4. Duplicate permission check
    handleSave checks edit permission, and the button also checks it via v-if. This is acceptable, but a computed property would avoid duplication:

    const canEditDocumentStrategy = computed(() =>
  knowledgeDocumentStrategyPermission('documentStrategyEdit')
)

    Then use it in both places.

  5. Minor existing issue
    max-w-200 is not a standard Tailwind utility unless custom-configured. Verify it resolves correctly.

Overall, the rename itself looks internally consistent in the shown diff. The main risks are the workspace-shared read behavior and ensuring the new permission identifiers exist everywhere.

Loading