Skip to content

Commit e5d39aa

Browse files
authored
Merge pull request #38 from initializ/skills/gh
feat: add GitHub API query tools and per-tool PII exemptions
2 parents 8a3a303 + 19c8f9e commit e5d39aa

16 files changed

Lines changed: 798 additions & 15 deletions

docs/hooks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ hooks.Register(engine.BeforeToolExec, func(ctx context.Context, hctx *engine.Hoo
7777

7878
`AfterToolExec` hooks can modify `hctx.ToolOutput` to redact sensitive content before it enters the LLM context. The agent loop reads back `ToolOutput` from the `HookContext` after all hooks fire.
7979

80-
The runner registers a guardrail hook that scans tool output for secrets and PII patterns. See [Tool Output Scanning](security/guardrails.md#tool-output-scanning) for details.
80+
The runner registers a guardrail hook that scans tool output for secrets and PII patterns. The hook passes `hctx.ToolName` to the guardrail engine, enabling per-tool exemptions via `allow_tools` config. See [Tool Output Scanning](security/guardrails.md#tool-output-scanning) for details.
8181

8282
```go
8383
hooks.Register(engine.AfterToolExec, func(ctx context.Context, hctx *engine.HookContext) error {

docs/security/guardrails.md

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ All four built-in guardrails (`content_filter`, `no_pii`, `jailbreak_protection`
7575

7676
## Tool Output Scanning
7777

78-
The guardrail engine scans tool output via an `AfterToolExec` hook, catching secrets and PII before they enter the LLM context or outbound messages.
78+
The guardrail engine scans tool output via an `AfterToolExec` hook, catching secrets and PII before they enter the LLM context or outbound messages. The hook passes the tool name to enable per-tool exemptions (see [Per-Tool PII Exemptions](#per-tool-pii-exemptions) below).
7979

8080
| Guardrail | What it detects in tool output |
8181
|-----------|-------------------------------|
@@ -86,11 +86,44 @@ The guardrail engine scans tool output via an `AfterToolExec` hook, catching sec
8686

8787
| Mode | Behavior |
8888
|------|----------|
89-
| `enforce` | Returns a generic error (`"tool output blocked by content policy"`), blocking the result from entering the LLM context. The error message intentionally omits which guardrail matched to avoid leaking security internals to the LLM or channel. |
89+
| `enforce` | Returns an error identifying the guardrail that triggered (e.g., `"tool output blocked by no_pii guardrail (PII detected in output)"`), blocking the result from entering the LLM context. |
9090
| `warn` | Replaces matched patterns with `[REDACTED]`, logs a warning, and allows the redacted output through |
9191

9292
The hook writes the redacted text back to `HookContext.ToolOutput`, which the agent loop reads after all hooks fire. This is backwards-compatible — existing hooks that don't modify `ToolOutput` leave it unchanged.
9393

94+
### Per-Tool PII Exemptions
95+
96+
Some tools legitimately return PII as part of their function (e.g., `github_get_user` returning public email addresses). The `allow_tools` config option lets specific tools bypass a guardrail entirely.
97+
98+
```json
99+
{
100+
"guardrails": [
101+
{
102+
"type": "no_pii",
103+
"config": {
104+
"allow_tools": [
105+
"github_get_user",
106+
"github_pr_author_profiles",
107+
"github_stargazer_profiles",
108+
"file_create",
109+
"code_agent_write",
110+
"code_agent_edit"
111+
]
112+
}
113+
}
114+
]
115+
}
116+
```
117+
118+
**Key behaviors:**
119+
120+
| Behavior | Detail |
121+
|----------|--------|
122+
| Per-guardrail scope | `allow_tools` on `no_pii` does **not** bypass `no_secrets` — each guardrail has its own allowlist |
123+
| Write tools included | `file_create`, `code_agent_write`, and `code_agent_edit` are included because they echo back content the LLM already has — blocking the echo is redundant |
124+
| Default config | The default policy scaffold pre-configures `allow_tools` for GitHub profile tools and write tools |
125+
| Custom overrides | Override via `policy-scaffold.json` to add or remove tools from the allowlist |
126+
94127
## Path Containment
95128

96129
The `cli_execute` tool confines filesystem path arguments to the agent's working directory. This prevents social-engineering attacks where an LLM is tricked into listing or reading files outside the project.

docs/skills.md

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ forge skills list --tags kubernetes,incident-response
163163

164164
| Skill | Icon | Category | Description | Scripts |
165165
|-------|------|----------|-------------|---------|
166-
| `github` | 🐙 | developer | Clone repos, create issues/PRs, and manage git workflows | `github-clone.sh`, `github-checkout.sh`, `github-commit.sh`, `github-push.sh`, `github-create-pr.sh`, `github-status.sh` |
166+
| `github` | 🐙 | developer | Clone repos, create issues/PRs, query GitHub API, and manage git workflows | `github-clone.sh`, `github-checkout.sh`, `github-commit.sh`, `github-push.sh`, `github-create-pr.sh`, `github-status.sh`, `github-list-prs.sh`, `github-get-user.sh`, `github-list-stargazers.sh`, `github-list-forks.sh`, `github-pr-author-profiles.sh`, `github-stargazer-profiles.sh` |
167167
| `code-agent` | 🤖 | developer | Autonomous code generation, modification, and project scaffolding | — (builtin tools) |
168168
| `weather` | 🌤️ | utilities | Get weather data for a location | — (binary-backed) |
169169
| `tavily-search` | 🔍 | research | Search the web using Tavily AI search API | `tavily-search.sh` |
@@ -365,7 +365,7 @@ The `github` skill provides a complete git + GitHub workflow through script-back
365365
forge skills add github
366366
```
367367

368-
This registers eight tools:
368+
This registers fourteen tools:
369369

370370
| Tool | Purpose |
371371
|------|---------|
@@ -377,9 +377,19 @@ This registers eight tools:
377377
| `github_create_pr` | Create a pull request |
378378
| `github_create_issue` | Create a GitHub issue |
379379
| `github_list_issues` | List open issues for a repository |
380+
| `github_list_prs` | List pull requests with state filter and pagination |
381+
| `github_get_user` | Get a GitHub user's public profile |
382+
| `github_list_stargazers` | List stargazers for a repository with pagination |
383+
| `github_list_forks` | List forks of a repository with pagination |
384+
| `github_pr_author_profiles` | List PR authors and fetch their full profiles (compound 2-step) |
385+
| `github_stargazer_profiles` | List stargazers and fetch their full profiles (compound 2-step) |
380386

381387
**Workflow:** Clone → explore → edit → status → commit → push → create PR. The skill's system prompt enforces this sequence and prevents raw `git` commands via `cli_execute`.
382388

389+
**Pagination:** List tools (`github_list_prs`, `github_list_stargazers`, `github_list_forks`, `github_pr_author_profiles`, `github_stargazer_profiles`) support `page` (1-based) and `per_page` (default 30, max 100) parameters. Responses include `pagination.has_next_page` to indicate more results are available.
390+
391+
**PII exemption:** Profile-returning tools (`github_get_user`, `github_pr_author_profiles`, `github_stargazer_profiles`) are pre-configured in the default policy scaffold's `no_pii` `allow_tools` list, so they can return public profile data (emails, bios) without triggering PII guardrails. See [Per-Tool PII Exemptions](security/guardrails.md#per-tool-pii-exemptions).
392+
383393
Requires: `gh`, `git`, `jq`. Optional: `GH_TOKEN`. Egress: `api.github.com`, `github.com`.
384394

385395
### Code-Agent Skill

forge-cli/runtime/guardrails_loader.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,19 @@ func DefaultPolicyScaffold() *agentspec.PolicyScaffold {
3636
Type: "content_filter",
3737
Config: map[string]any{"enabled": true},
3838
},
39-
{Type: "no_pii"},
39+
{
40+
Type: "no_pii",
41+
Config: map[string]any{
42+
"allow_tools": []any{
43+
"github_get_user",
44+
"github_pr_author_profiles",
45+
"github_stargazer_profiles",
46+
"file_create",
47+
"code_agent_write",
48+
"code_agent_edit",
49+
},
50+
},
51+
},
4052
{Type: "jailbreak_protection"},
4153
{Type: "no_secrets"},
4254
},

forge-cli/runtime/runner.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1474,7 +1474,7 @@ func (r *Runner) registerGuardrailHooks(hooks *coreruntime.HookRegistry, guardra
14741474
if hctx.ToolOutput == "" {
14751475
return nil
14761476
}
1477-
redacted, err := guardrails.CheckToolOutput(hctx.ToolOutput)
1477+
redacted, err := guardrails.CheckToolOutput(hctx.ToolName, hctx.ToolOutput)
14781478
if err != nil {
14791479
return err
14801480
}

forge-core/runtime/guardrails.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,20 +252,30 @@ func (g *GuardrailEngine) checkNoSecrets(text string) error {
252252
// because tool outputs are internal (sent to the LLM, not the user) and
253253
// blocking would kill the entire agent session. Search tools routinely find
254254
// code containing API key patterns in test files, config examples, etc.
255-
func (g *GuardrailEngine) CheckToolOutput(text string) (string, error) {
255+
//
256+
// The toolName parameter enables per-tool PII exemptions: if a guardrail's
257+
// config contains "allow_tools" (a list of tool names), tools in that list
258+
// skip the corresponding check. This lets tools like github_get_user return
259+
// public profile data (emails, bios) without triggering PII blocks.
260+
func (g *GuardrailEngine) CheckToolOutput(toolName, text string) (string, error) {
256261
if text == "" {
257262
return text, nil
258263
}
259264

260265
for _, gr := range g.scaffold.Guardrails {
266+
// Check if this tool is in the guardrail's allow_tools list.
267+
if g.toolAllowed(toolName, gr) {
268+
continue
269+
}
270+
261271
switch gr.Type {
262272
case "no_secrets":
263273
for _, re := range secretPatterns {
264274
if !re.MatchString(text) {
265275
continue
266276
}
267277
if g.enforce {
268-
return "", fmt.Errorf("tool output blocked by content policy")
278+
return "", fmt.Errorf("tool output blocked by no_secrets guardrail (secret/credential detected in output)")
269279
}
270280
text = re.ReplaceAllString(text, "[REDACTED]")
271281
g.logger.Warn("guardrail redaction", map[string]any{
@@ -295,7 +305,7 @@ func (g *GuardrailEngine) CheckToolOutput(text string) (string, error) {
295305
continue
296306
}
297307
if g.enforce {
298-
return "", fmt.Errorf("tool output blocked by content policy")
308+
return "", fmt.Errorf("tool output blocked by no_pii guardrail (PII detected in output)")
299309
}
300310
// Warn mode: redact only validated matches
301311
if p.validate != nil {
@@ -322,6 +332,27 @@ func (g *GuardrailEngine) CheckToolOutput(text string) (string, error) {
322332
return text, nil
323333
}
324334

335+
// toolAllowed checks whether toolName is in the guardrail's "allow_tools" config list.
336+
func (g *GuardrailEngine) toolAllowed(toolName string, gr agentspec.Guardrail) bool {
337+
if toolName == "" || gr.Config == nil {
338+
return false
339+
}
340+
allowRaw, ok := gr.Config["allow_tools"]
341+
if !ok {
342+
return false
343+
}
344+
list, ok := allowRaw.([]any)
345+
if !ok {
346+
return false
347+
}
348+
for _, v := range list {
349+
if s, ok := v.(string); ok && s == toolName {
350+
return true
351+
}
352+
}
353+
return false
354+
}
355+
325356
// --- PII Validators ---
326357
// Ported from the reference guardrails library to reduce false positives.
327358

forge-core/runtime/guardrails_test.go

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func TestCheckToolOutput_RedactsWithValidation(t *testing.T) {
202202
}, false, logger) // warn mode
203203

204204
// Valid SSN should be redacted
205-
out, err := g.CheckToolOutput("SSN is 456-78-9012")
205+
out, err := g.CheckToolOutput("some_tool", "SSN is 456-78-9012")
206206
if err != nil {
207207
t.Fatalf("unexpected error: %v", err)
208208
}
@@ -211,7 +211,7 @@ func TestCheckToolOutput_RedactsWithValidation(t *testing.T) {
211211
}
212212

213213
// Invalid SSN (area 000) should NOT be redacted
214-
out, err = g.CheckToolOutput("code 000-12-3456 here")
214+
out, err = g.CheckToolOutput("some_tool", "code 000-12-3456 here")
215215
if err != nil {
216216
t.Fatalf("unexpected error: %v", err)
217217
}
@@ -228,7 +228,7 @@ func TestCheckToolOutput_K8sBytesNotBlocked(t *testing.T) {
228228

229229
// K8s memory byte counts should not trigger PII detection
230230
k8sOutput := `{"memory": "4294967296", "cpu": "2000m", "pods": "110", "allocatable_memory": "3221225472"}`
231-
out, err := g.CheckToolOutput(k8sOutput)
231+
out, err := g.CheckToolOutput("some_tool", k8sOutput)
232232
if err != nil {
233233
t.Fatalf("k8s output blocked as PII: %v", err)
234234
}
@@ -243,10 +243,94 @@ func TestCheckToolOutput_EnforceBlocksValidPII(t *testing.T) {
243243
Guardrails: []agentspec.Guardrail{{Type: "no_pii"}},
244244
}, true, logger) // enforce mode
245245

246-
_, err := g.CheckToolOutput("SSN: 456-78-9012")
246+
_, err := g.CheckToolOutput("some_tool", "SSN: 456-78-9012")
247247
if err == nil {
248248
t.Error("expected enforce mode to block valid SSN")
249249
}
250+
if !strings.Contains(err.Error(), "no_pii") {
251+
t.Errorf("expected error to mention no_pii guardrail, got: %v", err)
252+
}
253+
}
254+
255+
func TestCheckToolOutput_AllowToolsBypassesPII(t *testing.T) {
256+
logger := &testLogger{}
257+
g := NewGuardrailEngine(&agentspec.PolicyScaffold{
258+
Guardrails: []agentspec.Guardrail{
259+
{
260+
Type: "no_pii",
261+
Config: map[string]any{
262+
"allow_tools": []any{"github_get_user", "github_pr_author_profiles"},
263+
},
264+
},
265+
},
266+
}, true, logger) // enforce mode
267+
268+
// Allowed tool should pass through with PII
269+
out, err := g.CheckToolOutput("github_get_user", `{"email": "user@example.com"}`)
270+
if err != nil {
271+
t.Fatalf("allowed tool should not be blocked: %v", err)
272+
}
273+
if !strings.Contains(out, "user@example.com") {
274+
t.Error("expected email to pass through for allowed tool")
275+
}
276+
277+
// Non-allowed tool should still be blocked
278+
_, err = g.CheckToolOutput("some_other_tool", `{"email": "user@example.com"}`)
279+
if err == nil {
280+
t.Error("expected non-allowed tool to be blocked for PII")
281+
}
282+
}
283+
284+
func TestCheckToolOutput_AllowToolsOnlyAffectsConfiguredGuardrail(t *testing.T) {
285+
logger := &testLogger{}
286+
g := NewGuardrailEngine(&agentspec.PolicyScaffold{
287+
Guardrails: []agentspec.Guardrail{
288+
{Type: "no_secrets"}, // no allow_tools — applies to all tools
289+
{
290+
Type: "no_pii",
291+
Config: map[string]any{
292+
"allow_tools": []any{"github_get_user"},
293+
},
294+
},
295+
},
296+
}, true, logger)
297+
298+
// Allowed tool bypasses PII but NOT secrets
299+
_, err := g.CheckToolOutput("github_get_user", "token: ghp_abcdefghijklmnopqrstuvwxyz0123456789")
300+
if err == nil {
301+
t.Error("allow_tools for no_pii should not bypass no_secrets")
302+
}
303+
if !strings.Contains(err.Error(), "no_secrets") {
304+
t.Errorf("expected error to mention no_secrets, got: %v", err)
305+
}
306+
}
307+
308+
func TestCheckToolOutput_ErrorMessageMentionsGuardrailType(t *testing.T) {
309+
logger := &testLogger{}
310+
311+
// Test no_secrets error message
312+
g := NewGuardrailEngine(&agentspec.PolicyScaffold{
313+
Guardrails: []agentspec.Guardrail{{Type: "no_secrets"}},
314+
}, true, logger)
315+
_, err := g.CheckToolOutput("some_tool", "key: sk-ant-abcdefghijklmnopqrstuv")
316+
if err == nil {
317+
t.Fatal("expected error for secret")
318+
}
319+
if !strings.Contains(err.Error(), "no_secrets") {
320+
t.Errorf("expected error to mention no_secrets, got: %v", err)
321+
}
322+
323+
// Test no_pii error message
324+
g2 := NewGuardrailEngine(&agentspec.PolicyScaffold{
325+
Guardrails: []agentspec.Guardrail{{Type: "no_pii"}},
326+
}, true, logger)
327+
_, err = g2.CheckToolOutput("some_tool", "email: test@example.com")
328+
if err == nil {
329+
t.Fatal("expected error for PII")
330+
}
331+
if !strings.Contains(err.Error(), "no_pii") {
332+
t.Errorf("expected error to mention no_pii, got: %v", err)
333+
}
250334
}
251335

252336
// --- CheckOutbound message tests ---

forge-core/runtime/loop.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,9 @@ func toolPhase(name string) workflowPhase {
672672
switch name {
673673
case "github_clone", "code_agent_scaffold", "github_checkout":
674674
return phaseSetup
675-
case "code_agent_read", "grep_search", "glob_search", "directory_tree", "read_skill", "github_status":
675+
case "code_agent_read", "grep_search", "glob_search", "directory_tree", "read_skill", "github_status",
676+
"github_list_prs", "github_get_user", "github_list_stargazers", "github_list_forks",
677+
"github_pr_author_profiles", "github_stargazer_profiles":
676678
return phaseExplore
677679
case "code_agent_edit", "code_agent_write", "code_agent_patch", "file_create", "code_agent_run":
678680
return phaseEdit

forge-core/runtime/loop_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,12 @@ func TestToolPhaseClassification(t *testing.T) {
368368
{"directory_tree", phaseExplore},
369369
{"read_skill", phaseExplore},
370370
{"github_status", phaseExplore},
371+
{"github_list_prs", phaseExplore},
372+
{"github_get_user", phaseExplore},
373+
{"github_list_stargazers", phaseExplore},
374+
{"github_list_forks", phaseExplore},
375+
{"github_pr_author_profiles", phaseExplore},
376+
{"github_stargazer_profiles", phaseExplore},
371377
{"code_agent_edit", phaseEdit},
372378
{"code_agent_write", phaseEdit},
373379
{"code_agent_patch", phaseEdit},

0 commit comments

Comments
 (0)