feat: add audit pattern harness and security patterns example

[moul]

Summary

Moves the audit pattern harness into this repo and keeps the public builder-facing updates alongside it.

This adds:

• misc/audit-pattern-harness: a small audit pattern harness with YAML expected records, sanitized fixtures, gno test execution, and Markdown/JSON reports via cmd/auditpattern
• eight initial pattern slices:
  • cur.Previous() before cur.IsCurrent()
  • raw Render(path) markdown output
  • Render output that depends on map iteration order
  • OriginSend() without an IsUserCall() guard
  • OriginCaller() used as authorization identity
  • caller-supplied callbacks accepted by realm APIs
  • interfaces that expose cur realm
  • exported pointers or pointer getters for mutable state
• gno.land/r/docs/security_patterns: a compact example realm showing authenticated mutators and safe Render output
• security-guide updates for caller-identity selection, Render output, official markdown sanitizing guidance, and OriginCaller authorization
• Effective Gno storage guidance kept concise, with fuller tree/seqid examples moved to gno-data-structures.md
• community-packages.md: a disclaimer-backed place for non-official helper packages such as moul/md, ulist, addrset, and fifo
• a docs index link

The additional patterns were calibrated from local examples/ plus checked-out Gno contract repos including GnoSwap, Akkadia, gno-ibc, and gno-mcp fixtures, then reduced into sanitized fixtures rather than copying source.

Validation

• go test ./... in misc/audit-pattern-harness
• make run GO=/home/russel/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.25.9.linux-amd64/bin/go AUDIT_PATTERN_FLAGS='-gno-bin /tmp/gno-audit-harness-gno' in misc/audit-pattern-harness with GNOROOT=/home/russel/p/gh/gnoland/gno
• gno test . in examples/gno.land/r/docs/security_patterns with GNOROOT=/home/russel/p/gh/gnoland/gno
• gno test . in examples/gno.land/r/docs/home with GNOROOT=/home/russel/p/gh/gnoland/gno
• git diff --check

Assisted-By: OpenAI Codex

[Gno2D2]

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

• IGNORE the bot requirements for this PR (force green CI check)

Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)

☑️ Contributor Actions:

1. Fix any issues flagged by automated checks.
2. Follow the Contributor Checklist to ensure your PR is ready for review.
   • Add new tests, or document why they are unnecessary.
   • Provide clear examples/screenshots, if necessary.
   • Update documentation, if required.
   • Ensure no breaking changes, or include BREAKING CHANGE notes.
   • Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

• Report a bug with the bot.
• View the bot’s configuration file.

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: moul/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

• Any user with comment edit permission

[moul]

I'm looking for individuals to help me challenge and improve the following:

• the overall concept/mechanism of this PR
• the current documentation

The goal is to release a V1 and then iterate. We aim to use the documentation increasingly to start writing and reviewing realms based on it. We'll definitely want to update gnoverse/gno-mcp.

[Villaquiranm]

This won't catch bad formatted files:

func GetVault() *Vault { ✅ hit
func GetVault()*Vault{ ❌ miss
func (vault *Vault) GetVault()*Vault{ ❌ miss

maybe use a regex also here ? I don't know if it makes sense also to catch for methods
var exportedFuncPointerRE = regexp.MustCompile(^func\s+(?:([^)])\s+)?[A-Z]\w\s*(.)\s*)
works also for methods

[moul]

I will add a formatting preliminary step.

[moul]

Done in 278bd24 — added a gofmt pre-step (go/format.Source) that normalizes each .gno file before the matchers run, so func GetVault()*Vault{ is canonicalized to func GetVault() *Vault { and gets caught. Falls back to the raw bytes if a fixture is intentionally unparseable. Added TestRuleNormalizesFormatting covering your exact example.

Methods (func (v *Vault) Get() *Vault) are still out of scope here — the exported-pointer matcher only targets top-level funcs — but the formatting fix removes the spacing-based misses.

[davd-gzl]

Ran the reference realm and the harness guards on 34ac1e7; each rejects the attacker case it claims to.

Full review: https://github.com/samouraiworld/gno-agent-workspace/blob/main/reviews/pr/5xxx/5835-audit-pattern-harness/2-34ac1e7cd/review_claude-opus-4-8_davd-gzl.md

The new audit-pattern-harness module is absent from the ci-dir-misc.yml matrix, so its Go tests and TestAgentPatternContract never run in CI. Add it to the matrix.

---

First review pass, I need to do a second one.

[moul]

Fixed in 02640e1 — row now says authenticate the immediate caller with cur.Previous() under a cur.IsCurrent() guard, and notes IsUserCall() is for the payment row only.

[moul]

Fixed in 02640e1 — swapped to sanitize.InlineText. Verified gno test still passes: InlineText escapes [ ] ( ) > (the inlineEscapeSet), so the existing escaping assertions hold.

[moul]

Fixed in 02640e1 — dropped the stale runtime. qualifier; the snippet now uses bare OriginCaller() like the rest of §5.8.

[moul]

Addressed in ee38075 — TestAgentPatternContract now asserts each vulnerable hit's text contains the rule's detection signal (.Previous(), OriginSend(), range , *, …), so a rule rewritten to flag a coincidental line (an import, etc.) fails. Full per-line golden assertions across all rules would be stronger still — left as a follow-up.

[moul]

Fixed in ee38075 — added an intro note explaining the (quarantined) marker and flagged all 7 quarantined-only packages (collection, bitset, pausable, query, forms, blog, chess).

[moul]

Fixed in ee38075 — set gno = "0.9".

[moul]

Fixed in ee38075 — added codeLines() (go/scanner) that blanks string/char-literal contents and comment bodies before matching, so a } (or keyword/call) inside a literal can no longer fool brace-depth tracking or the substring checks. Hits are still reported against the original source. Added TestBraceInStringNoFalsePositive (your repro). The 5-scanners→2 refactor I left as a follow-up.

[davd-gzl]

The interface-realm-param and callback-param slices show the bad pattern, a realm handed to caller-supplied code, but not the safe one: threading cur through your own concrete /p/ functions, which daokit's interrealm-v2 port needs. The danger is specifically a caller-supplied func or interface value, because a realm token grants authority only while cur.IsCurrent() holds. Spell that out in one sentence, or readers avoid passing realms to /p/ at all and lose the safe threading pattern.

[moul]

Good point, not done yet. The safe pattern — thread your own cur through concrete /p/ functions; a realm token only grants authority while cur.IsCurrent() holds — deserves an explicit sentence (the danger is specifically a caller-supplied func/interface). Ill add it to the security guide alongside the callback/interface families; open to where youd prefer it.

[davd-gzl]

It would suit the best at §5.3, right under the "never invoke a caller-supplied function/interface value" rule.

[davd-gzl]

Verified on e8281bc.

Follow-up, not blocking: AGENTS.md:98 positions the harness as a regular sanity check, but the docs name it only in one hard-to-spot bullet. Surface it somewhere discoverable like the quickstart.

Full review: https://github.com/samouraiworld/gno-agent-workspace/blob/main/reviews/pr/5xxx/5835-audit-pattern-harness/3-e8281bcbe/review_claude-opus-4-8_davd-gzl.md

---

After this, I think this LGTM

[davd-gzl]

On code that is not already gofmt-clean, the reported file:line and text come from the gofmt-reformatted buffer, not the on-disk file, so a hit can point at the wrong line. Committed and editor-formatted realms are already gofmt-clean, so this only bites on unformatted input such as generated or pasted code.

[davd-gzl]

The patterns are implicit: Render prints only the admin, message, and path, and only assertAdmin has a comment. Add a short explanation in Render, which gnoweb renders as the page body, or comments on the code, saying what each pattern is and why it is safer: the cur.IsCurrent() guard, cur.Previous().Address() over OriginCaller(), and InlineText on user text.

[davd-gzl]

A backtick in path closes the manual code span, and path reaches Render as arbitrary bytes. Backslash-escaping does not help, since a backslash is literal inside a code span. md.InlineCode(path) is the safe primitive.

repro

# from a local clone of gnolang/gno:
gh pr checkout 5835 -R gnolang/gno
go build -o /tmp/gno ./gnovm/cmd/gno
export GNOROOT=$(pwd)
cd examples/gno.land/r/docs/security_patterns
cat > zz_bt_test.gno <<'GO'
package securitypatterns

import "testing"

func TestBacktickPath(t *testing.T) { t.Logf("%q", Render("a`b")) }
GO
/tmp/gno test -v -run TestBacktickPath .
rm zz_bt_test.gno

zz_bt_test.go:5: "# Security Patterns\n\n...\n\nPath: `a\`b`\n"

The ````` is the escape, but inside a code span the backslash is literal, so the span closes at the user's backtick and b lands outside it with a dangling backtick.

[davd-gzl]

After the whitespace-normalization fix, render_map_iteration still matches range +name as a substring with no right word boundary, so a map named scores flags range scoresList, an unrelated slice. A word boundary after the map name removes it.

repro

# from a local clone of gnolang/gno:
gh pr checkout 5835 -R gnolang/gno
cd misc/audit-pattern-harness
cat > internal/auditpattern/zz_mapfp_test.go <<'GO'
package auditpattern

import (
	"os"
	"path/filepath"
	"testing"
)

func TestMapIterSubstringFP(t *testing.T) {
	d := t.TempDir()
	src := "package x\n\nvar scores = map[string]int{}\nvar scoresList = []int{}\n\nfunc Render(path string) string {\n\tfor _, v := range scoresList {\n\t\t_ = v\n\t}\n\treturn \"\"\n}\n"
	os.WriteFile(filepath.Join(d, "a.gno"), []byte(src), 0o644)
	hits, _ := RunRule("render_map_iteration", d)
	if len(hits) != 0 {
		t.Fatalf("unrelated slice flagged: %+v", hits)
	}
}
GO
go test -count=1 -run TestMapIterSubstringFP ./internal/auditpattern/
rm internal/auditpattern/zz_mapfp_test.go

--- FAIL: TestMapIterSubstringFP (0.00s)
    unrelated slice flagged: [{File:a.gno Line:7 Text:for _, v := range scoresList {}]
FAIL

[davd-gzl]

origin_caller_auth flags every OriginCaller() read, including a benign emit("actor", unsafe.OriginCaller().String()) with no comparison and no auth. exported_pointer_leak also flags func NewVault() *Vault, a normal constructor whose returned pointer is to a fresh object, not shared state, so there is nothing to leak. Neither rule has a README "Known limitations" entry, so tighten the heuristics or extend the caveats.

repro

# from a local clone of gnolang/gno:
gh pr checkout 5835 -R gnolang/gno
cd misc/audit-pattern-harness
cat > internal/auditpattern/zz_fpfn_test.go <<'GO'
package auditpattern

import (
	"os"
	"path/filepath"
	"testing"
)

func write(t *testing.T, src string) string {
	d := t.TempDir()
	os.WriteFile(filepath.Join(d, "a.gno"), []byte(src), 0o644)
	return d
}

func TestOriginCallerBenignFP(t *testing.T) {
	src := "package x\n\nimport \"chain/runtime/unsafe\"\n\nfunc Log() {\n\temit(\"actor\", unsafe.OriginCaller().String())\n}\n"
	t.Logf("origin_caller_auth benign-read hits: %+v", mustRun(t, "origin_caller_auth", write(t, src)))
}

func TestConstructorFP(t *testing.T) {
	src := "package x\n\ntype Vault struct{ B int }\n\nfunc NewVault() *Vault {\n\treturn &Vault{}\n}\n"
	t.Logf("exported_pointer_leak constructor hits: %+v", mustRun(t, "exported_pointer_leak", write(t, src)))
}

func mustRun(t *testing.T, rule, dir string) []Hit { h, _ := RunRule(rule, dir); return h }
GO
go test -count=1 -v -run 'TestOriginCallerBenignFP|TestConstructorFP' ./internal/auditpattern/
rm internal/auditpattern/zz_fpfn_test.go

    origin_caller_auth benign-read hits: [{File:a.gno Line:6 Text:emit("actor", unsafe.OriginCaller().String())}]
    exported_pointer_leak constructor hits: [{File:a.gno Line:5 Text:func NewVault() *Vault {}]

[moul]

Addressed in 3700f767f.

I tightened both heuristics and added regression tests:

• origin_caller_auth now requires a direct == or != comparison around OriginCaller(), so benign logging like emit("actor", unsafe.OriginCaller().String()) is ignored.
• exported_pointer_leak now ignores fresh constructors shaped like NewX() *X { return &X{} }, while keeping hits for exported pointer globals and pointer getters that can alias package state.
• Added TestOriginCallerBenignRead and TestExportedPointerLeakIgnoresFreshConstructor.
• Added README caveats for the remaining heuristic limits.

[davd-gzl]

The docs point a human at the harness but never document it: how to run it, what it outputs, how to read a result. This matters even agent-first: when a human does not understand a flagged result, the agent needs human docs to redirect them to. Document it in the harness README and link this bullet to it.

[moul]

Addressed in 3700f767f.

I expanded misc/audit-pattern-harness/README.md with:

• how to run the harness;
• how to read the markdown report;
• what gno test, pattern-hit counts, and hit lines mean;
• known limitations for the tightened heuristic rules.

I also changed this security-guide bullet to link directly to the README, so humans and agents have a concrete place to land.

[davd-gzl]

None of the 16 fixtures carry a comment, so a human reading a vulnerable/fixed pair learns nothing about what is wrong or what the fix changed. Add a one-line comment marking the vulnerable construct and the fix, here the cur.Previous() read with no cur.IsCurrent() guard.

[moul]

Addressed in 3700f767f.

I added one-line teaching comments to every vulnerable/fixed fixture pair, including this one. For current-guard, the vulnerable fixture now calls out the unguarded cur.Previous() read, and the fixed fixture calls out the cur.IsCurrent() guard before reading the previous frame.

[moul]

Pushed 3700f767f to address the latest review feedback.

Changes included:

• surfaced misc/audit-pattern-harness in the builder quickstart;
• linked the security guide directly to the harness README;
• expanded the harness README with run/output interpretation notes;
• added one-line explanatory comments to vulnerable/fixed fixtures;
• tightened origin_caller_auth to avoid benign OriginCaller() logging reads;
• tightened exported_pointer_leak to ignore fresh constructors like NewVault() *Vault { return &Vault{} };
• added regression tests for both reported false positives.

Local checks run:

• go test ./... from misc/audit-pattern-harness
• make run AUDIT_PATTERN_FLAGS='-format json' from misc/audit-pattern-harness
• make lint from docs

[moul]

Two new patterns surfaced while analyzing agent-generated contracts against the audit harness — both are missing from the current pattern set.

Missing pattern: unsafe.PreviousRealm() alongside cur realm

Scanning 5 contracts deployed by an AI agent on test13, every one used chain/runtime/unsafe.PreviousRealm() instead of cur.Previous() — the cur realm parameter was accepted but never used and the IsCurrent() guard was skipped entirely. This is the single most common mistake in agent-generated Gno code right now.

Suggested pattern: flag any file that (a) imports chain/runtime/unsafe AND (b) declares a crossing function func F(cur realm, ...). The fix is to delete the unsafe import and replace unsafe.PreviousRealm().Address() with cur.IsCurrent() guard + cur.Previous().Address().

Missing pattern: /p/-type with mutation methods returned as pointer

When a realm stores a /p/-type instance (e.g. *avl.Tree) and returns a pointer to it, readonly taint on the pointer does not block method dispatch — borrow rule #2 fires first and the writes inside mutation methods commit under the realm's authority. This applies even when all fields of the /p/ type are unexported; the exported methods are the attack surface.

The exported-pointer-leak pattern catches func GetXxx() *T but only for receiver-less pointer returns. This new pattern is specifically about /p/-typed returns whose type has known mutation methods (Set, Remove, etc.).

A sub-case: exported pointer fields on /p/ structs (type Container struct { Items *avl.Tree }) — returning *Container gives the same indirect mutation path through container.Items.Set(...).

---

These cases are now documented in docs/resources/gno-security-guide.md §5.1a/5.1b/5.8 and docs/resources/gno-ai-contract-review.md (cases 8–9) in #5880. Adding fixtures + expected YAML here would complete the coverage.

[thehowl]

I imagine from this latest comment that there's a bit of work yet to be done on this PR? (I can review if not)