security: add SECURITY.md, drop the /bin/sh -c footgun, document the real threat model

[metif12]

Summary

Audit of the reported command-injection surface, done against the current tree rather than the 2023 reports, which cite a src/ layout that no longer exists.

  git/git.v
- pub fn Git.exec_shell(command string) os.Result {
- 	return os.exec(['/bin/sh', '-c', command])
- }
+ // NOTE: there is deliberately no `exec_shell` wrapper here. Handing a
+ // concatenated command string to `/bin/sh -c` is how command injection gets
+ // introduced, and it is never needed: every caller below builds an argument
+ // vector and passes it to `os.exec`, so arguments are never re-parsed by a
+ // shell. Add an argument-array helper rather than a string one.

Plus SECURITY.md, which the reporter of #246 asked for in 2023 and which the repository still does not have.

Evidence

The 2023 report's cited files do not exist. src/repo_routes.v, src/commit_routes.v, and src/repo_service.v are all gone; the tree was restructured and the helpers are now in git/git.v and repo/repo.v. So the report cannot be acted on as written and has to be re-derived.

Every external command runs through argv. All four os.exec call sites in git/git.v pass an array:

pub fn Git.exec(args []string) os.Result {
	mut git_args := ['git']
	git_args << args
	return os.exec(git_args)
}

// Git.exec_in_dir_with_env runs `git -C <dir> <args...>` without going through a
// shell, with `extra_env` added on top of the inherited environment. Use this
// instead of building a shell string when any argument or environment value is
// user-controlled (file paths, branch names, commit messages, author names),
// since arguments are passed directly to git and never re-parsed by /bin/sh.

split_command (git/git.v:218) is purely lexical — word splitting and unquoting, with no command substitution, variable expansion, or globbing — and its result goes to argv.

exec_shell had zero callers. A repository-wide search for exec_shell returns exactly one match, its own definition:

D:\MyProjects\gitly\git\git.v:
  Line 33: pub fn Git.exec_shell(command string) os.Result {

Private-repository visibility (#247) is already fixed. repo/repo.v:1796:

fn (app &App) can_read_repo(ctx Context, repo Repo) bool {
	if repo.is_public {
		return true
	}
	return ctx.logged_in && app.user_has_repo_read_access(ctx.user.id, repo)
}

That check is called at 43 sites across every repo-scoped read route, including tree() in repo/repo_routes.v:753, which is the route the report described (/:username/:repo_name). Regression tests already exist in api/endpoints_test.v:919 (test_api_v1_private_repo_visibility_from_other_user, which asserts 404 for a private repo from another authenticated user, and 200 for the owner) and pr_authorization_test.v.

Build still passes after the removal:

$ v -d sqlite -o gitly.exe .
EXIT=0
True

Merge Danger

Door: Two-way. exec_shell had no callers, so removing it cannot change behaviour; the rest is comments and a new file. Reverting is clean.

Blast Radius: Small, but one thing deserves scrutiny.

I deliberately did not fix the residual issue I found, and named it in the code instead. Repo.git() guards against & and ;, which reads like a shell guard but is not one, because there is no shell. The actual residual risk is argument injection: a ref name beginning with - can still be interpreted by git as an option, and split_command splits on whitespace, so a ref containing a space becomes several arguments. The callers that interpolate a caller-supplied ref are pr_routes.v:859 / pr_routes.v:886 (log ${base}..${head}, diff ${base}...${head}) and repo/branch.v:67.

Fixing that properly means moving those callers onto Git.exec_in_dir_with_env with an argument array and a -- separator, which is a behavioural change I cannot test here beyond a compile. It is a separate change and should be reviewed on its own merits, not smuggled into a cleanup.