fix(security): harden apish - path traversal, CORS, auth, and cleanup by thbkrkr · Pull Request #2 · thbkrkr/go-apish

thbkrkr · 2026-06-06T00:01:24Z

Summary

Security: prevent path traversal in script execution; fix wildcard+credentials CORS combo; compare API key in constant time; gate /docker behind a flag then remove it entirely; configurable basic-auth user with no default API key; warn when running unauthenticated
Fixes: handle errors in POST handler; return 204 for favicon; treat any stat error as missing index; log and exit on server error; derive /ls URL scheme from the request
Refactor: expose /version without auth; deduplicate GET/POST exec handlers; unify logging on logrus; single static walk with real error propagation; drop favicon route and slim CORS headers
Build: multi-stage Dockerfile with Go 1.25; drop dead release target
Docs: KISS README rewrite with inline flag comments, real curl output, layout tree; example scripts build JSON safely with jq
Tests: add coverage for path traversal, POST, invalid JSON, /ls, script failure, wrong credentials; fix broken apiDir path after restructuring; simplify HTTP helpers
Helm: minimal chart (Deployment + Service) with configurable image, port, auth flags; helm-* Makefile targets using apish as release name
Build: bump Go 1.25 → 1.26.4 and alpine 3.20 → 3.23; tag example image as krkr/apish:example; add push and port-forward targets to example Makefile

Test plan

go test ./... passes
helm template renders without errors (make helm-render)
./go-apish -apiDir=example serves scripts and static files
Auth is required when -password is set; server warns when it is not
make -C example build && make -C example run serves the example image on port 80

Validate the wildcard request path resolves within ApiDir before executing, rejecting escapes like /api/../../../tmp/evil that could run arbitrary .sh files anywhere on the filesystem.

The /docker endpoint runs arbitrary 'docker run' commands, which is effectively root on the host. Make it opt-in (off by default) and replace strings.Split with a quote-aware tokenizer so quoted args with spaces are preserved.

Access-Control-Allow-Origin: * together with Allow-Credentials: true is rejected by browsers. Remove the credentials header and add X-Auth to the allowed request headers so header auth works cross-origin.

Use crypto/subtle.ConstantTimeCompare for the X-Auth header check to avoid timing side channels.

When -password is unset the server is fully open; emit a loud startup warning so this isn't silently the default.

The canonical examples interpolated values straight into the JSON output; $1 in param.sh could break the JSON (or worse in a script that evaluates it). Use 'jq -n --arg/--argjson' so values are always correctly typed and escaped.

PostExecScript discarded the Start()/Wait() errors with '_ =' and then checked a never-assigned err, so failing scripts fell through to a confusing 'Invalid JSON' 400. Use cmd.Run(), check its error, and capture stderr for the error log.

c.JSON(200, nil) sent the literal 'null' as a favicon. Return 204 instead, and update the test expectation.

Previously a non-NotExist error (e.g. permissions) returned true and triggered a redirect to a file that can't be read. Only treat a nil error as present.

The bare 'for { s.ListenAndServe() }' silently restarted the server on error (busy-looping when the port is unavailable) and never logged why. Log the error and exit. Also clarify the startup log measures setup time.

Move /version out of the auth group so health/version checks work even when -password is set.

Extract shared scriptPath validation, existence check (resolve) and JSON-emitting execution (run). ExecScript and PostExecScript now differ only in how the command is built (query arg vs stdin body). GET also gains stderr capture for consistency.

Replace the standard 'log' package and fmt.Printf logging in main.go, router.go and handlers with logrus (already used by docker.go), and drop the redundant [info]/[error]/[warn] prefixes now that levels convey them.

Store ApiDir as string instead of *string in ExecHandler/LsHandler. The flag value is resolved once at Router() construction, removing handler dependence on a mutable global pointer.

Collapse the two _static walks into one pass that splits HTML pages from other files, and propagate walk errors (the walk funcs previously always returned nil, making the error checks dead code). The optional _static directory is skipped when absent.

fileToUrl hardcoded http://, producing wrong links behind TLS/a proxy. Build the base URL from the request scheme (TLS or X-Forwarded-Proto) and pass it in. Also switch strings.Replace(-1) to strings.ReplaceAll.

Add a -user flag for the basic-auth username (was a hardcoded global) and default -apiKey to empty. An empty key now disables X-Auth header auth entirely so it can't accidentally match a missing header.

Replace the golang:1.6.2 + alpine:3.7 build (which can't build a go 1.25 module) with a multi-stage Dockerfile on golang:1.25-alpine + alpine:3.20. The image now builds itself; 'make binary' builds locally and 'make build' builds the image, both injecting git commit/date via ldflags.

The 'make release' target called ./release.sh, which is not in the repo. Remove it; image publishing is covered by 'make push'.

Add a flags table, endpoint reference, the JSON output contract, the authentication model, and a security section covering -password and the -enableDocker host-access risk.

Add a handler unit test asserting scriptPath rejects escapes from ApiDir, plus HTTP tests for POST execution, invalid-JSON 400, /ls listing, and a path-traversal request. Fix TestBase for the /version no-auth route and the authenticated / -> /s redirect.

Key the test.BasicAuth literals (go vet warned on unkeyed fields) and remove the unused 'reader' variable and its 'io' import.

The docker run endpoint is no longer used. Remove the handler, the -enableDocker flag, its route, the example docker script/page, and the related README docs.

GOMAXPROCS already defaults to NumCPU since Go 1.5, so the manual call was a no-op. Drop the function and the runtime import.

Rename Get2 to GetWithKey with a plain string key, replace deprecated ioutil.ReadAll with io.ReadAll, make the unexported do() fail fast with t.Fatal, and close the response body.

Remove the /favicon.ico 204 convenience route (let it 404 like any other unknown path) and trim the CORS middleware to the origin, methods and headers this API actually uses.

Drop the most complex example script; the simpler date/param/post examples are enough to show the JSON contract.

Inline the ~60-line HTTP helper package as unexported helpers in the single test file, dropping the separate package and the ServerURL/ PrefixURL globals (helpers use the test server directly).

cdn.ractivejs.org is defunct and the http:// URL was blocked as mixed content on HTTPS pages. Point to a pinned jsDelivr build over HTTPS.

Move exec.go, ls.go, cors.go, auth.go (and exec_test.go) to the root as package main and drop the cross-package imports. For a project this small a single package is simpler than three.

- Go source files moved to app/ so the root stays clean - example/api/{test,time,_static} flattened to example/{test,time,_static} - Added fail.sh with proper shebang for clean 500 error output - Updated README: KISS rewrite with inline flag comments, curl examples with real output, and layout tree showing dir→URL mapping Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Use filepath.Abs for ApiDir in ListResources (avoids ReplaceAll mangling the leading ../), point the tests and README run example at ../example / example now that the api/ level is gone.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Dockerfile: Go 1.25 → 1.26.4, alpine 3.20 → 3.23 - example/Dockerfile: tag as krkr/apish:example, add entrypoint CMD - example/Makefile: push target, fix run image name, add port-forward - helm/values.yaml: default image tag to example, pullPolicy Always Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

thbkrkr added 30 commits June 2, 2026 17:38

fix(security): prevent path traversal in script execution

32259f9

Validate the wildcard request path resolves within ApiDir before executing, rejecting escapes like /api/../../../tmp/evil that could run arbitrary .sh files anywhere on the filesystem.

fix(security): gate /docker behind -enableDocker flag

5d4ad6f

The /docker endpoint runs arbitrary 'docker run' commands, which is effectively root on the host. Make it opt-in (off by default) and replace strings.Split with a quote-aware tokenizer so quoted args with spaces are preserved.

fix(security): drop invalid wildcard+credentials CORS combo

cd3a949

Access-Control-Allow-Origin: * together with Allow-Credentials: true is rejected by browsers. Remove the credentials header and add X-Auth to the allowed request headers so header auth works cross-origin.

fix(security): compare API key in constant time

d8e4b37

Use crypto/subtle.ConstantTimeCompare for the X-Auth header check to avoid timing side channels.

fix(security): warn when running without authentication

ca898bf

When -password is unset the server is fully open; emit a loud startup warning so this isn't silently the default.

fix: return 204 No Content for favicon

4f03acd

c.JSON(200, nil) sent the literal 'null' as a favicon. Return 204 instead, and update the test expectation.

fix: treat any stat error as missing index in indexExists

467766d

Previously a non-NotExist error (e.g. permissions) returned true and triggered a redirect to a file that can't be read. Only treat a nil error as present.

fix: log and exit on server error instead of busy-looping

8a95b79

The bare 'for { s.ListenAndServe() }' silently restarted the server on error (busy-looping when the port is unavailable) and never logged why. Log the error and exit. Also clarify the startup log measures setup time.

refactor: expose /version without authentication

81f8cb7

Move /version out of the auth group so health/version checks work even when -password is set.

refactor: unify logging on logrus

ae9e4e9

Replace the standard 'log' package and fmt.Printf logging in main.go, router.go and handlers with logrus (already used by docker.go), and drop the redundant [info]/[error]/[warn] prefixes now that levels convey them.

refactor: pass apiDir to handlers by value

67f1701

Store ApiDir as string instead of *string in ExecHandler/LsHandler. The flag value is resolved once at Router() construction, removing handler dependence on a mutable global pointer.

fix: derive listed URL scheme from the request

ad69a4b

fileToUrl hardcoded http://, producing wrong links behind TLS/a proxy. Build the base URL from the request scheme (TLS or X-Forwarded-Proto) and pass it in. Also switch strings.Replace(-1) to strings.ReplaceAll.

fix(security): configurable basic-auth user, no default API key

a77bedd

Add a -user flag for the basic-auth username (was a hardcoded global) and default -apiKey to empty. An empty key now disables X-Auth header auth entirely so it can't accidentally match a missing header.

build: drop release target referencing missing release.sh

49bd455

The 'make release' target called ./release.sh, which is not in the repo. Remove it; image publishing is covered by 'make push'.

docs: document flags, endpoints, auth and security in README

b1b0db2

Add a flags table, endpoint reference, the JSON output contract, the authentication model, and a security section covering -password and the -enableDocker host-access risk.

test: use keyed struct literals and drop dead reader var

06a9052

Key the test.BasicAuth literals (go vet warned on unkeyed fields) and remove the unused 'reader' variable and its 'io' import.

feat: remove the /docker endpoint

074ec6e

The docker run endpoint is no longer used. Remove the handler, the -enableDocker flag, its route, the example docker script/page, and the related README docs.

refactor: remove redundant ConfigRuntime

25b663a

GOMAXPROCS already defaults to NumCPU since Go 1.5, so the manual call was a no-op. Drop the function and the runtime import.

test: simplify HTTP test helpers

17bbcbf

Rename Get2 to GetWithKey with a plain string key, replace deprecated ioutil.ReadAll with io.ReadAll, make the unexported do() fail fast with t.Fatal, and close the response body.

docker

c17a135

refactor: drop favicon route and slim CORS headers

8489c7b

Remove the /favicon.ico 204 convenience route (let it 404 like any other unknown path) and trim the CORS middleware to the origin, methods and headers this API actually uses.

docs(example): remove inception self-calling demo

4541c2a

Drop the most complex example script; the simpler date/param/post examples are enough to show the JSON contract.

test: fold test/ helper package into main_test.go

9e54589

Inline the ~60-line HTTP helper package as unexported helpers in the single test file, dropping the separate package and the ServerURL/ PrefixURL globals (helpers use the test server directly).

docs(example): fix dead Ractive CDN URL in date.html

72349c7

cdn.ractivejs.org is defunct and the http:// URL was blocked as mixed content on HTTPS pages. Point to a pinned jsDelivr build over HTTPS.

thbkrkr and others added 6 commits June 2, 2026 23:26

refactor: flatten handlers and middlewares into the main package

cacfab6

Move exec.go, ls.go, cors.go, auth.go (and exec_test.go) to the root as package main and drop the cross-package imports. For a project this small a single package is simpler than three.

fix: align paths with flattened example/ layout

8807d56

Use filepath.Abs for ApiDir in ListResources (avoids ReplaceAll mangling the leading ../), point the tests and README run example at ../example / example now that the api/ level is gone.

feat: add helm chart and helm Makefile targets

39dfd2b

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: update PR description

aeaa7ca

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

thbkrkr force-pushed the review-fixes branch from 9897a35 to aeaa7ca Compare June 6, 2026 00:04

Navigation Menu

fix(security): harden apish - path traversal, CORS, auth, and cleanup - #2

fix(security): harden apish - path traversal, CORS, auth, and cleanup#2
thbkrkr wants to merge 36 commits into
masterfrom
review-fixes

thbkrkr commented Jun 6, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

thbkrkr commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

thbkrkr commented Jun 6, 2026 •

edited

Loading