docs/security-attack-vector-mapping.md

@@ -0,0 +1,67 @@
+# Security Attack Vector Mapping
+
+`CODEX.md` was not present in `/workspace`, so this review follows [`CLAUDE.md`](../CLAUDE.md) plus the repository conventions already in force. No fixes were made.
+
+## Scope
+
+- Target: all non-test external input entry points in the library and `cmd/forgegen`
+- Output format: `function`, `file:line`, `input source`, `what it flows into`, `current validation`, `potential attack vector`
+- Line ranges below are grouped when a row covers multiple adjacent methods with the same behaviour
+
+## Cross-Cutting Sinks
+
+These internal paths are reached by most public API methods and are the main places where untrusted data actually crosses the trust boundary.
+
+| Function | File:line | Input source | What it flows into | Current validation | Potential attack vector |
+|---|---|---|---|---|---|
+| `Client.doJSON` | `client.go:216-260` | Caller-supplied `path` and `body`; remote Forgejo JSON/body/headers | `baseURL + path`, `json.Marshal(body)`, `http.NewRequestWithContext`, `http.Client.Do`, `json.Decoder.Decode(out)` | Request method is fixed by caller; JSON marshal failure stops request; `http.NewRequestWithContext` validates URL syntax; no body-size cap; no response-size cap; no `DisallowUnknownFields` | Arbitrary request path within configured host; query/fragment injection if upstream callers pass unescaped path segments; request/response memory exhaustion; malicious Forgejo can return oversized or schema-confusing JSON |
+| `Client.parseError` | `client.go:263-285` | Remote error body | Up to 1024 bytes copied into `APIError.Message` | Size capped at 1 KiB; best-effort JSON parse only | Server-controlled error text can propagate to logs/UI and support log-forging or misleading diagnostics |
+| `Client.updateRateLimit` | `client.go:287-297` | Remote rate-limit headers | `Client.rateLimit` cache | Parses with `Atoi`/`ParseInt`; conversion errors ignored | Malicious or buggy server can poison rate-limit telemetry and mislead callers about throttling state |
+| `Client.PostRaw` | `client.go:137-176` | Caller-supplied `path` and `body`; remote raw response body | `http.Client.Do` then `io.ReadAll(resp.Body)` | JSON marshal only; no response-size cap; no content-type check | Memory exhaustion on large responses; raw HTML/text returned to caller can become XSS/content-injection if rendered downstream |
+| `Client.GetRaw` | `client.go:180-208` | Caller-supplied `path`; remote raw response body | `http.Client.Do` then `io.ReadAll(resp.Body)` | No response-size cap; no content-type check | Memory exhaustion on large file responses; untrusted bytes flow straight to caller |
+| `ListPage` | `pagination.go:32-70` | Caller-supplied `path`, `query`, `opts`; remote `X-Total-Count` and JSON list body | `url.Parse(path)`, query encoding, `Client.doJSON` | `Page`/`Limit` clamped to minimum `1`; query values encoded with `url.Values`; no max page/limit cap | Arbitrary in-host endpoint access if caller controls `path`; malicious server can lie in `X-Total-Count` and increase work or confuse pagination state |
+| `ListAll`, `ListIter` | `pagination.go:73-113` | Remote pagination metadata and page contents | Repeated `ListPage` calls until `HasMore == false` | No max page bound; loop stops only when server indicates completion or returns a short page | Request amplification / unbounded pagination against a malicious or buggy server |
+| `ResolvePath` | `params.go:13-17` | Caller-supplied path template and placeholder values | `strings.ReplaceAll` with `url.PathEscape(v)` | Placeholder values are path-escaped | Prevents slash/query breakout from placeholder values, but does not verify required placeholders are present or that the template itself is trusted |
+
+## Entry Points
+
+| Function | File:line | Input source | What it flows into | Current validation | Potential attack vector |
+|---|---|---|---|---|---|
+| `ResolveConfig` | `config.go:21-35` | Environment variables `FORGE_URL`, `FORGE_TOKEN`; caller-supplied flag values | Returned `url`/`token` consumed by `NewForgeFromConfig`/`NewClient` | Priority only: flags override env, empty URL falls back to `http://localhost:3000` | If env/flags are attacker-controlled, requests and auth token can be redirected to an attacker-chosen Forgejo instance, including cleartext `http://` |
+| `NewForgeFromConfig`, `NewForge`, `NewClient` | `config.go:39-48`; `forge.go:29-51`; `client.go:81-95` | Caller-supplied `url`, `token`, `opts` | `Client.baseURL`, `Authorization` header, service wiring | `NewForgeFromConfig` only checks token is non-empty; `NewClient` trims trailing `/`; redirects disabled by default | SSRF/token exfiltration to attacker-controlled host; accidental cleartext transport; malformed base URL causes runtime request failures instead of early rejection |
+| `WithHTTPClient`, `WithUserAgent` | `client.go:50-56` | Caller-supplied `*http.Client` and User-Agent string | Overrides transport/TLS/proxy behaviour and `User-Agent` header | No validation | Untrusted caller can supply insecure transport settings or proxy all traffic; arbitrary UA content is forwarded to downstream logs/telemetry |
+| `Client.Get`, `Client.Post`, `Client.Patch`, `Client.Put`, `Client.Delete`, `Client.DeleteWithBody` | `client.go:99-130` | Caller-supplied `path` and optional request body; remote JSON/error responses | Thin wrappers around `Client.doJSON` | No local path validation; body only has JSON-marshalling checks | Direct low-level escape hatch to any in-host API path; if upstream code passes unescaped path segments, this layer will forward them verbatim |
+| `NewResource` | `resource.go:20-30` | Caller-supplied item path template | Stored `path`/`collection` later used by CRUD methods | Only strips a trailing pure placeholder segment when deriving `collection` | If a library consumer builds `Resource` from untrusted templates, later CRUD calls can target arbitrary API paths |
+| `Resource.List`, `Resource.ListAll`, `Resource.Iter`, `Resource.Get`, `Resource.Create`, `Resource.Update`, `Resource.Delete` as surfaced by `RepoService`, `IssueService`, `PullService`, `OrgService`, `UserService`, `TeamService`, `BranchService`, `ReleaseService`, `WebhookService` | `resource.go:34-77`; `repos.go:17-19`; `issues.go:18-20`; `pulls.go:18-20`; `orgs.go:18-20`; `users.go:18-20`; `teams.go:18-20`; `branches.go:18-20`; `releases.go:18-20`; `webhooks.go:19-20` | Caller-supplied `Params` placeholder values and typed request bodies; remote JSON responses | `ResolvePath` then `ListPage`/`ListAll`/`ListIter` or `Client.Get/Post/Patch/Delete` | Placeholder values path-escaped; request bodies are type-shaped only; no required-param check; no semantic validation | Lower path-injection risk than the `fmt.Sprintf` call sites, but missing placeholders can still hit wrong paths; oversized/malformed bodies depend entirely on server-side validation; all response-size/pagination risks from the shared sinks still apply |
+| `CommitService.List`, `CommitService.ListAll`, `CommitService.Iter`, `CommitService.Get` | `commits.go:29-48` | Caller-supplied `Params` (`owner`, `repo`, `sha`/`ref`) | `ResolvePath` into `ListPage`/`ListAll`/`ListIter`/`Client.Get` | Uses `ResolvePath`; no semantic validation of `sha`/`ref` | Same as generic `Resource` methods: path breakout is mitigated, but placeholder omissions and unbounded remote responses remain possible |
+| `AdminService.EditUser`, `AdminService.DeleteUser`, `AdminService.RenameUser`, `AdminService.RunCron`, `AdminService.AdoptRepo` | `admin.go:41-57`; `admin.go:69-86` | Caller-supplied `username`, `newName`, `task`, `owner`, `repo`, `opts` | `ResolvePath` then `Client.Patch/Post/Delete` | Path values escaped by `ResolvePath`; `opts` for `EditUser` is an unrestricted `map[string]any` | Path breakout is mitigated, but `EditUser` forwards arbitrary JSON fields and all methods rely on server-side validation for semantics and size |
+| Fixed-path network fetch/update methods: `AdminService.ListUsers`, `AdminService.IterUsers`, `AdminService.CreateUser`, `AdminService.ListOrgs`, `AdminService.IterOrgs`, `AdminService.ListCron`, `AdminService.IterCron`, `AdminService.GenerateRunnerToken`, `RepoService.ListUserRepos`, `RepoService.IterUserRepos`, `OrgService.ListMyOrgs`, `OrgService.IterMyOrgs`, `UserService.GetCurrent`, `NotificationService.List`, `NotificationService.Iter`, `NotificationService.MarkRead`, `NotificationService.GetThread`, `NotificationService.MarkThreadRead`, `MiscService.ListLicenses`, `MiscService.ListGitignoreTemplates`, `MiscService.GetNodeInfo`, `MiscService.GetVersion` | `admin.go:22-39`; `admin.go:59-91`; `repos.go:34-39`; `orgs.go:61-66`; `users.go:25-31`; `notifications.go:22-59`; `misc.go:34-81` | Remote Forgejo responses; for `CreateUser`, caller-supplied typed body; for thread methods, numeric IDs | Fixed API paths or `%d` path formatting into `Client` or pagination helpers | No local validation beyond Go types and integer formatting | Main risk is trust in remote server responses plus unbounded pagination/JSON decode; `CreateUser` also forwards request body without field-level validation |
+| `MiscService.RenderMarkdown` | `misc.go:24-31` | Caller-supplied markdown `text` and `mode`; remote raw HTML response | `types.MarkdownOption` -> `Client.PostRaw` -> raw string return | Struct-shaped body only; no output sanitisation | If caller renders the returned HTML directly, untrusted markdown or compromised server output can become XSS/content injection; large responses can exhaust memory |
+| `ActionsService.ListRepoSecrets`, `ActionsService.IterRepoSecrets`, `ActionsService.CreateRepoSecret`, `ActionsService.DeleteRepoSecret`, `ActionsService.ListRepoVariables`, `ActionsService.IterRepoVariables`, `ActionsService.CreateRepoVariable`, `ActionsService.DeleteRepoVariable`, `ActionsService.ListOrgSecrets`, `ActionsService.IterOrgSecrets`, `ActionsService.ListOrgVariables`, `ActionsService.IterOrgVariables`, `ActionsService.DispatchWorkflow` | `actions.go:23-99` | Caller-supplied `owner`, `repo`, `org`, `name`, `workflow`, `data`, `value`, `opts`; remote JSON responses | `fmt.Sprintf` path building -> `Client.Get/Put/Post/Delete` or pagination helpers | No path escaping; bodies are either simple structs/maps or raw strings | Path/endpoint confusion via `/`, `?`, `#`, or encoded separators in names/orgs/repos/workflow IDs; `DispatchWorkflow` forwards arbitrary JSON fields via `map[string]any`; pagination/response risks still apply |
+| `BranchService.ListBranchProtections`, `BranchService.IterBranchProtections`, `BranchService.GetBranchProtection`, `BranchService.CreateBranchProtection`, `BranchService.EditBranchProtection`, `BranchService.DeleteBranchProtection` | `branches.go:25-67` | Caller-supplied `owner`, `repo`, `name`, typed option structs; remote JSON responses | `fmt.Sprintf` path building -> `Client.Get/Post/Patch/Delete` or pagination helpers | No path escaping; body shape only | Unescaped branch-protection names, owners, or repos can alter the targeted API path; body content is unchecked locally |
+| `CommitService.GetCombinedStatus`, `CommitService.ListStatuses`, `CommitService.CreateStatus`, `CommitService.GetNote` | `commits.go:53-83` | Caller-supplied `owner`, `repo`, `ref`/`sha`, typed status body; remote JSON responses | `fmt.Sprintf` path building -> `Client.Get/Post` | No path escaping; body shape only | Branch names/refs containing `/`, `?`, or `#` can retarget requests or make legitimate refs unreachable; body fields rely on server validation |
+| `ContentService.GetFile`, `ContentService.CreateFile`, `ContentService.UpdateFile`, `ContentService.DeleteFile`, `ContentService.GetRawFile` | `contents.go:21-57` | Caller-supplied `owner`, `repo`, `filepath`, typed file option structs; remote JSON/raw file responses | `fmt.Sprintf` path building -> `Client.Get/Post/Put/DeleteWithBody/GetRaw` | No path escaping; nested `filepath` is passed through verbatim | `filepath` is intentionally free-form but reserved characters such as `?`, `#`, and ambiguous percent-encoding can still alter routing/query semantics; raw file reads have no size cap |
+| `IssueService.Pin`, `IssueService.Unpin`, `IssueService.SetDeadline`, `IssueService.AddReaction`, `IssueService.DeleteReaction`, `IssueService.StartStopwatch`, `IssueService.StopStopwatch`, `IssueService.AddLabels`, `IssueService.RemoveLabel`, `IssueService.ListComments`, `IssueService.IterComments`, `IssueService.CreateComment` | `issues.go:25-102` | Caller-supplied `owner`, `repo`, numeric IDs, deadline/reaction/body/label list; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping for `owner`/`repo`; request bodies are raw strings, slices, or simple structs | Owner/repo path breakout; comment/reaction/deadline values go straight upstream with no local validation or size limits |
+| `LabelService.ListRepoLabels`, `LabelService.IterRepoLabels`, `LabelService.GetRepoLabel`, `LabelService.CreateRepoLabel`, `LabelService.EditRepoLabel`, `LabelService.DeleteRepoLabel`, `LabelService.ListOrgLabels`, `LabelService.IterOrgLabels`, `LabelService.CreateOrgLabel` | `labels.go:22-89` | Caller-supplied `owner`, `repo`, `org`, numeric IDs, typed option structs; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping for string path segments; body shape only | Path/endpoint confusion through unescaped owner/repo/org values; no local validation on label metadata |
+| `MilestoneService.ListAll`, `MilestoneService.Get`, `MilestoneService.Create` | `milestones.go:20-44` | Caller-supplied `params["owner"]`, `params["repo"]`, `owner`, `repo`, typed create body; remote JSON responses | `fmt.Sprintf` path building -> pagination helpers or `Client.Get/Post` | No path escaping; missing map keys become empty path segments | Path manipulation and silent empty-segment requests if `owner`/`repo` are absent or attacker-controlled; no local validation on milestone body |
+| `MiscService.GetLicense`, `MiscService.GetGitignoreTemplate` | `misc.go:43-69` | Caller-supplied template name; remote JSON responses | `fmt.Sprintf` path building -> `Client.Get` | No path escaping | Template names with reserved characters can alter the request path/query |
+| `NotificationService.ListRepo`, `NotificationService.IterRepo` | `notifications.go:32-40` | Caller-supplied `owner`, `repo`; remote JSON responses | `fmt.Sprintf` path building -> pagination helpers | No path escaping | Owner/repo path breakout or query injection against repo-scoped notification endpoints |
+| `OrgService.ListMembers`, `OrgService.IterMembers`, `OrgService.AddMember`, `OrgService.RemoveMember`, `OrgService.ListUserOrgs`, `OrgService.IterUserOrgs` | `orgs.go:25-55` | Caller-supplied `org`, `username`; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping | Unescaped org/user names can alter the intended endpoint path |
+| `PackageService.List`, `PackageService.Iter`, `PackageService.Get`, `PackageService.Delete`, `PackageService.ListFiles`, `PackageService.IterFiles` | `packages.go:22-61` | Caller-supplied `owner`, `pkgType`, `name`, `version`; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping | Package coordinates with `/`, `?`, `#`, or crafted percent-encoding can retarget the request or make legitimate versions unreachable |
+| `PullService.Merge`, `PullService.Update`, `PullService.ListReviews`, `PullService.IterReviews`, `PullService.SubmitReview`, `PullService.DismissReview`, `PullService.UndismissReview` | `pulls.go:25-69` | Caller-supplied `owner`, `repo`, numeric IDs, `method`, `review`, `msg`; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping for string path segments; `SubmitReview` forwards unrestricted `map[string]any` | Owner/repo path breakout; `Merge` method value is unchecked locally; `SubmitReview` can forward arbitrary JSON fields to the API |
+| `RepoService.ListOrgRepos`, `RepoService.IterOrgRepos`, `RepoService.Fork`, `RepoService.Transfer`, `RepoService.AcceptTransfer`, `RepoService.RejectTransfer`, `RepoService.MirrorSync` | `repos.go:24-73` | Caller-supplied `org`, `owner`, `repo`, transfer `opts`; remote JSON responses | String concatenation / `map[string]string` / `map[string]any` -> `Client.*` and pagination helpers | No path escaping; `Transfer` body unrestricted; `Fork` only conditionally adds `organization` body field | Owner/repo/org path breakout; `Transfer` is a direct arbitrary-JSON passthrough; body size/field semantics rely on server enforcement |
+| `ReleaseService.GetByTag`, `ReleaseService.DeleteByTag`, `ReleaseService.ListAssets`, `ReleaseService.IterAssets`, `ReleaseService.GetAsset`, `ReleaseService.DeleteAsset` | `releases.go:25-68` | Caller-supplied `owner`, `repo`, `tag`, numeric IDs; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping for string segments | Tag names or repo identifiers with reserved characters can alter endpoint targeting |
+| `TeamService.ListMembers`, `TeamService.IterMembers`, `TeamService.ListRepos`, `TeamService.IterRepos`, `TeamService.AddMember`, `TeamService.RemoveMember`, `TeamService.AddRepo`, `TeamService.RemoveRepo`, `TeamService.ListOrgTeams`, `TeamService.IterOrgTeams` | `teams.go:25-79` | Caller-supplied `teamID`, `username`, `org`, `repo`; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | Numeric IDs are formatted safely; string path segments are unescaped | Numeric-only methods are lower risk, but the member/repo/org methods still allow path confusion through unescaped string segments |
+| `UserService.ListFollowers`, `UserService.IterFollowers`, `UserService.ListFollowing`, `UserService.IterFollowing`, `UserService.Follow`, `UserService.Unfollow`, `UserService.ListStarred`, `UserService.IterStarred`, `UserService.Star`, `UserService.Unstar` | `users.go:34-88` | Caller-supplied `username`, `owner`, `repo`; remote JSON responses | `fmt.Sprintf` path building -> `Client.*` and pagination helpers | No path escaping | Username/owner/repo values can alter targeted endpoints or inject query/fragment data |
+| `WebhookService.TestHook`, `WebhookService.ListOrgHooks`, `WebhookService.IterOrgHooks` | `webhooks.go:26-40` | Caller-supplied `owner`, `repo`, `org`, numeric `id`; remote JSON responses | `fmt.Sprintf` path building -> `Client.Post` and pagination helpers | Numeric ID formatting only for `id`; no path escaping for string segments | Owner/repo/org values can alter endpoint targeting; organisation hook enumeration inherits pagination risks |
+| `WikiService.ListPages`, `WikiService.GetPage`, `WikiService.CreatePage`, `WikiService.EditPage`, `WikiService.DeletePage` | `wiki.go:21-61` | Caller-supplied `owner`, `repo`, `pageName`, typed page body; remote JSON responses | `fmt.Sprintf` path building -> `Client.Get/Post/Patch/Delete` | No path escaping | `pageName` is free-form and may legitimately contain slashes, so reserved characters can change routing or query semantics; page content/body is forwarded without local validation |
+| `main` | `cmd/forgegen/main.go:9-30` | CLI flags `-spec`, `-out` | `LoadSpec`, `ExtractTypes`, `DetectCRUDPairs`, `Generate` | Standard `flag` parsing only | Untrusted operator input can point the generator at sensitive local files or arbitrary output directories |
+| `LoadSpec` | `cmd/forgegen/parser.go:73-84` | Caller/CLI-supplied file path and file contents | `coreio.Local.Read(path)` then `json.Unmarshal` into `Spec` | Parse failure only; entire file read into memory | Arbitrary local file read if path is attacker-controlled; large or malformed JSON can exhaust memory or CPU |
+| `ExtractTypes`, `DetectCRUDPairs` | `cmd/forgegen/parser.go:86-147` | Parsed, potentially untrusted Swagger definitions and metadata | In-memory `GoType`/`CRUDPair` model used by `Generate` | No schema hardening beyond basic JSON types; names/comments/enum values are accepted as-is | Malicious spec can create huge intermediate structures or feed unsafe identifiers/strings into code generation |
+| `Generate` | `cmd/forgegen/generator.go:206-263` | Caller-supplied `outDir`; type names/comments/enum values derived from the spec | `EnsureDir(outDir)`, `filepath.Join(outDir, file+".go")`, Go source template, `coreio.Local.Write` | Output file names are limited by `classifyType`; generated source content is mostly unsanitised spec data | Arbitrary directory creation/file overwrite if `outDir` is attacker-controlled; malicious spec strings can break generated Go syntax or inject unexpected source content into generated files |
+
+## Highest-Risk Patterns
+
+- Unescaped `fmt.Sprintf`/string-concatenated paths are the dominant library issue pattern. Most custom service methods trust raw `owner`, `repo`, `org`, `username`, `name`, `tag`, `workflow`, `pageName`, and similar values.
+- The `Resource`/`ResolvePath` path family is materially safer because it uses `url.PathEscape`, but it still performs no semantic validation and still inherits unbounded response/pagination behaviour.
+- `cmd/forgegen` assumes the spec file is trusted. If the swagger input is attacker-controlled, the generator can read arbitrary files, write arbitrary directories, and emit attacker-influenced Go source.

forge.go

@@ -22,6 +22,7 @@ type Forge struct {
 	Wiki          *WikiService
 	Misc          *MiscService
 	Commits       *CommitService
+	Milestones    *MilestoneService
 }
 
 // NewForge creates a new Forge client.
@@ -46,6 +47,7 @@ func NewForge(url, token string, opts ...Option) *Forge {
 	f.Wiki = newWikiService(c)
 	f.Misc = newMiscService(c)
 	f.Commits = newCommitService(c)
+	f.Milestones = newMilestoneService(c)
 	return f
 }
 

milestones.go

@@ -0,0 +1,43 @@
+package forge
+
+import (
+	"context"
+	"fmt"
+
+	"dappco.re/go/core/forge/types"
+)
+
+// MilestoneService handles repository milestones.
+type MilestoneService struct {
+	client *Client
+}
+
+func newMilestoneService(c *Client) *MilestoneService {
+	return &MilestoneService{client: c}
+}
+
+// ListAll returns all milestones for a repository.
+func (s *MilestoneService) ListAll(ctx context.Context, params Params) ([]types.Milestone, error) {
+	path := fmt.Sprintf("/api/v1/repos/%s/%s/milestones", params["owner"], params["repo"])
+	return ListAll[types.Milestone](ctx, s.client, path, nil)
+}
+
+// Get returns a single milestone by ID.
+func (s *MilestoneService) Get(ctx context.Context, owner, repo string, id int64) (*types.Milestone, error) {
+	path := fmt.Sprintf("/api/v1/repos/%s/%s/milestones/%d", owner, repo, id)
+	var out types.Milestone
+	if err := s.client.Get(ctx, path, &out); err != nil {
+		return nil, err
+	}
+	return &out, nil
+}
+
+// Create creates a new milestone.
+func (s *MilestoneService) Create(ctx context.Context, owner, repo string, opts *types.CreateMilestoneOption) (*types.Milestone, error) {
+	path := fmt.Sprintf("/api/v1/repos/%s/%s/milestones", owner, repo)
+	var out types.Milestone
+	if err := s.client.Post(ctx, path, opts, &out); err != nil {
+		return nil, err
+	}
+	return &out, nil
+}

types/issue.go

@@ -8,7 +8,7 @@ import "time"
 // CreateIssueCommentOption — CreateIssueCommentOption options for creating a comment on an issue
 type CreateIssueCommentOption struct {
 	Body string `json:"body"`
-	Updated time.Time `json:"updated_at,omitempty"`
+	Updated *time.Time `json:"updated_at,omitempty"`
 }
 
 // CreateIssueOption — CreateIssueOption options to create one issue