Skip to content

fix: return error instead of panic on missing policy type in LoadPolicyArray - #1757

Draft
zjncs wants to merge 1 commit into
apache:masterfrom
zjncs:fix/load-policy-array-empty-ptype
Draft

fix: return error instead of panic on missing policy type in LoadPolicyArray#1757
zjncs wants to merge 1 commit into
apache:masterfrom
zjncs:fix/load-policy-array-empty-ptype

Conversation

@zjncs

@zjncs zjncs commented Sep 4, 2026

Copy link
Copy Markdown

Description

persist.LoadPolicyArray panics when the rule has no policy type:

func LoadPolicyArray(rule []string, m model.Model) error {
	key := rule[0]
	sec := key[:1]
	...

Two reachable vectors:

  • a policy line with a stray leading comma (e.g. , alice, data1 — an easy hand-editing typo) is tokenized by LoadPolicyLine's csv reader into ["", "alice", "data1"], so key is "" and key[:1] panics with slice bounds out of range [:1] with length 0;
  • adapters calling the exported LoadPolicyArray directly (the documented helper for database rows) with an empty slice panic on rule[0].

Either way, NewEnforcer/LoadPolicy crashes the process instead of returning an error, although every other malformed-line case (bad quoting, wrong rule size) already returns an error — see HasPolicyEx's "invalid policy rule size: ..." and commit a98973b improving LoadPolicyArray error handling.

Fix: return a descriptive error when the rule is empty or its policy type token is empty.

Testing

Added TestLoadPolicyArrayEmptyPtype (empty type token and empty rule) and TestLoadPolicyLineLeadingComma (line , alice, data1) in persist/persist_test.go.

  • Before the fix: both panic — panic: runtime error: slice bounds out of range [:1] with length 0 at persist/adapter.go:78
  • After the fix: both return an error (invalid policy rule: missing policy type)

All existing tests in persist still pass, the full go test ./... suite passes (8 packages), and go vet ./persist/ is clean.


This PR was prepared with AI assistance (GitHub Copilot / ZCode autonomous agent). All findings were verified manually against the codebase before submission.

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.

1 participant