fix(admin,cfg): promote adminTTL const to configurable cfg.AdminTokenTTL (TD-010)#7
Merged
Merged
Conversation
…TTL (TD-010) Delete the magic-number const `adminTTL = 300` in internal/admin/admin_svc.go and replace with an operator-configurable field driven by AA_ADMIN_TOKEN_TTL. Default preserved as a named const (defaultAdminTokenTTL = 300) so no magic number leaks into cfg.Load(). Operators tune admin JWT lifetime without recompiling. Production code: - internal/cfg/cfg.go: added AdminTokenTTL int field (seconds), named const defaultAdminTokenTTL, env var AA_ADMIN_TOKEN_TTL, inline doc comment - internal/admin/admin_svc.go: deleted const adminTTL, added tokenTTL field to AdminSvc, NewAdminSvc signature extended with tokenTTL int parameter, Authenticate() now issues with s.tokenTTL instead of the hardcoded const - cmd/broker/main.go: NewAdminSvc wiring passes c.AdminTokenTTL Tests: - internal/admin/admin_svc_test.go: added testAdminTokenTTL = 300 fixture const, updated both newTestAdminSvc helpers to pass it, updated assertion to compare against the fixture instead of the deleted const - internal/admin/admin_hdl_test.go: same pattern — both NewAdminSvc calls pass testAdminTokenTTL, assertion compares against it - internal/app/app_hdl_test.go: NewAdminSvc call updated (passes 300 literal; app tests don't care about the value) - internal/handler/handler_test.go: same — NewAdminSvc call updated - internal/cfg/cfg_test.go: added TestLoad_AdminTokenTTLDefault and TestLoad_AdminTokenTTLCustom covering default and override paths The existing assertion `resp.ExpiresIn != testAdminTokenTTL` in admin_svc_test is the unit-level equivalent of a config-matrix behavioral test for this field — it proves the full cfg.AdminTokenTTL -> AdminSvc.tokenTTL -> TknSvc.Issue -> claims.Exp-Iat flow at test time. Int seconds (not time.Duration): the existing TTL fields (DefaultTTL, MaxTTL, AppTokenTTL) all use int seconds. Adding one time.Duration field would create two conventions in the same cfg package and leak into every caller. A future cleanup (proposed TD-CFG-003) can migrate all TTL fields to time.Duration together — mixing conventions mid-stream would be worse than preserving the existing one. Validation: go build ./..., go vet ./..., gofmt -l, go test ./... all green across 15 packages (includes cmd/broker now that main.go is touched). Marks TD-010 RESOLVED in TECH-DEBT.md. Action plan: .plans/specs/2026-04-10-td-010-action-plan.md
devonartis
added a commit
that referenced
this pull request
Apr 10, 2026
Catches docs up with the configuration defaults landed in PR #6 (TD-TOKEN-001, TD-CFG-001, TD-CFG-002) and PR #7 (TD-010). Docs were previously teaching the old hardcoded brand values in example JWT payloads, SPIFFE URIs, filesystem paths, and env var defaults. Now they reflect the current defaults as shipped. Direct push to develop (not PR) per explicit user instruction — this is documentation chasing production defaults, not new code. No Go source changes. Sweep patterns (bulk sed across docs/**/*.{md,yaml,yml,svg}): - agentauth.local -> agentwrit.local (matches cfg.TrustDomain default) - agentauth.db -> data.db (matches cfg.DBPath default) - /etc/agentauth/ -> /etc/broker/ (matches cfg.configfile.go search path) - /var/lib/agentauth/ -> /var/lib/broker/ (consistent data-dir naming) - ~/.agentauth/ -> ~/.broker/ (matches cfg.configfile.go home fallback) - "iss": "agentauth" -> "iss": "agentwrit" (example JWT payloads) - "aud": "agentauth" -> "aud": "agentwrit" (example JWT payloads) - AgentAuth -> AgentWrit (brand prose) - agentauth (bare) -> agentwrit (brand prose) - AGENTAUTH -> AGENTWRIT (any uppercase) Preserved (not changed): - github.com/devonartis/agentauth — Go module path + GitHub clone URLs (getting-started-user.md lines 74, 104). Module path rename is gated on the GitHub repo rename, which is separate work. - CHANGELOG historical entries (not touched) - tests/*/evidence/*.md historical records (not touched — they're in tests/ not docs/, so the sweep didn't reach them anyway) Renames: - docs/agentauth-explained.md -> docs/agentwrit-explained.md Targeted updates: - docs/credential-model.md:154 — the "hardcoded adminTTL = 300, see TD-010" text replaced with the new reality: "5 minutes default, operator-configurable via AA_ADMIN_TOKEN_TTL." TD-010 is now RESOLVED in TECH-DEBT.md so the reference was stale. Scope: ~20 doc files + 1 file rename. No code, no tests, no scripts.
devonartis
added a commit
that referenced
this pull request
Apr 12, 2026
Includes: - PR #6: hardcoded identity audit fixes (TD-TOKEN-001/002/003, TD-CFG-001/002, TD-TEST-001) - PR #7: adminTTL → cfg.AdminTokenTTL (TD-010) - PR #8: aactl → awrit binary rename (TD-CLI-001) - PR #11: docs audit P1/P2 corrections + docs-layout-archon-style sweep - PR #12: runtime rebrand alignment — problemdetails URN, obs metrics, awrit init config path (TD-RUNTIME-001, TD-OBS-001, TD-CLI-002), GitHub repo rename agentauth → agentwrit, Go module path rename, README/CONTRIBUTING/docs sweep, CI brand-alignment gate - Direct-to-develop: brand sweep docs (684083b) Stripped 16 dev-only paths: MEMORY.md, FLOW.md, TECH-DEBT.md, MEMORY_ARCHIVE.md, CLAUDE.md, AGENTS.md, COWORK_SESSION.md, COWORK_DOCS_AUDIT.md, .plans/, .claude/, .agents/, audit/, adr/, .vscode/, .active_module, generate_pdf.py.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deletes the magic-number const
adminTTL = 300frominternal/admin/admin_svc.goand replaces it with an operator-configurable field driven byAA_ADMIN_TOKEN_TTL. Default preserved as a named const (defaultAdminTokenTTL = 300) incfg.goso no magic number leaks intoLoad().Resolves: TD-010 (marked RESOLVED in
TECH-DEBT.md).What changed
Production:
cfg.Cfg.AdminTokenTTL intadded, envAA_ADMIN_TOKEN_TTL, defaultdefaultAdminTokenTTL = 300(seconds)admin.NewAdminSvcsignature extended withtokenTTL intparameter; stored on the struct, used inAuthenticate()cmd/broker/main.gopassesc.AdminTokenTTLthroughTests:
admin_svc_test.go: addedtestAdminTokenTTL = 300fixture const, updated bothnewTestAdminSvc*helpers, updated assertionadmin_hdl_test.go: bothNewAdminSvccall sites updated, assertion updatedapp_hdl_test.go+handler_test.go:NewAdminSvccalls updated (orthogonal — these tests don't check TTL, just pass 300)cfg_test.go: addedTestLoad_AdminTokenTTLDefault+TestLoad_AdminTokenTTLCustomWhy
intseconds and nottime.DurationThe existing TTL fields (
DefaultTTL,MaxTTL,AppTokenTTL) all use int seconds. Adding onetime.Durationfield would create two conventions in the same cfg package and leak into every caller that passes the field through. A future cleanup (proposed TD-CFG-003) can migrate all TTL fields totime.Durationtogether — mixing conventions mid-stream would be worse than preserving the existing one.CI-matrix-via-unit-test
The existing
resp.ExpiresIn != testAdminTokenTTLassertion inadmin_svc_testis the unit-level equivalent of a config-matrix behavioral test for this field — it proves the fullcfg.AdminTokenTTL -> AdminSvc.tokenTTL -> TknSvc.Issue -> claims.Exp-Iatflow at test time. No new CI infrastructure needed for this particular field because it's all inside one package boundary.Test plan
go build ./...cleango vet ./...cleangofmt -lreturns emptygo test ./...— 15/15 packages pass locally (full suite, not just internal/)gates-passedaggregator greenScope boundaries
time.Duration(separate proposed TD-CFG-003)adminTTL = 300(e.g.docs/credential-model.md:154) — separate doc sweepAction plan:
.plans/specs/2026-04-10-td-010-action-plan.md