Skip to content

fix(config): honor settings.commandAuthorization in .gittensory.yml - #2385

Merged
JSONbored merged 4 commits into
mainfrom
claude/command-authorization-config-parser
Jul 1, 2026
Merged

fix(config): honor settings.commandAuthorization in .gittensory.yml#2385
JSONbored merged 4 commits into
mainfrom
claude/command-authorization-config-parser

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

What

.gittensory.yml.example documented settings.commandAuthorization as settable config-as-code, but FocusManifestSettings never included the field and parseSettingsOverride never read it — it was silently dropped, no warning, nothing. The field only actually took effect via the dashboard/API route (DB-persisted). This fails toward the safe side (DB/default policy silently continues), but it's a real parity gap against this repo's own .gittensory.yml > DB > defaults convention, and it's security-relevant: a maintainer restricting who can invoke /gate-override via yml would believe the restriction was live when it silently wasn't.

Fix

  • Added commandAuthorization to the Pick<RepositorySettings, ...> union in FocusManifestSettings (src/signals/focus-manifest.ts).
  • Wired settings.commandAuthorization into parseSettingsOverride using the existing normalizeCommandAuthorizationPolicy (src/settings/command-authorization.ts), folding its warnings into the manifest's warnings output. The normalizer always fills any unset/invalid field from DEFAULT_COMMAND_AUTHORIZATION_POLICY — the same secure default used when the DB has none — so once the key is present in yml it always yields a complete, safe policy, mirroring how settings.autoMaintain already works ("declares the full policy, defaults fill any unset field").
  • resolveEffectiveSettings's existing {...dbSettings, ...manifest.settings} spread needed no change — once out.commandAuthorization is set, it wholesale-overlays the DB value the same way every other settings field does; leaving the yml key unset leaves it untouched (DB stays authoritative).
  • Updated .gittensory.yml.example's doc comment for the field with a concrete example and a note on the invalid-shape fallback behavior.
  • settingsOverrideToJson (the cache round-trip serializer) needed no change — it's a generic spread of whatever's on FocusManifestSettings.

Tests

  • New test: valid settings.commandAuthorization parses, overlays a DB-stored policy via resolveEffectiveSettings, and an unset key leaves the DB policy untouched (never resets to defaults).
  • New test: an invalid top-level shape (e.g. a string) falls back to the secure built-in default with a visible warning; a spoofable role on a maintainer-only command (e.g. pr_author on gate-override) is clamped back to the default for that command specifically, with its own warning — the maintainer-only invariant survives a partially-valid override.
  • npx tsc --noEmit clean.
  • Scoped: focus-manifest.test.ts — 138 passed.
  • Regression sweep: all 16 other files importing src/signals/focus-manifest.ts — 373 passed.
  • Diff-range coverage-gap check on src/signals/focus-manifest.ts: fully covered.
  • Full unsharded npm run test:coverage: 5602 passed, 4 skipped (pre-existing/unrelated), 0 failed.
  • npm audit --audit-level=moderate: 0 vulnerabilities.

Advances #1936. Closes #2268.

FocusManifestSettings never included commandAuthorization and
parseSettingsOverride never read it, so a maintainer setting
settings.commandAuthorization in .gittensory.yml had it silently
dropped — the DB-stored (possibly more permissive) policy stayed in
effect with no warning, contradicting this repo's yml > DB > defaults
config-as-code convention for every other settings field.

Add commandAuthorization to FocusManifestSettings and parse it with
the existing normalizeCommandAuthorizationPolicy, folding its
warnings into the manifest output. The normalizer always fills unset
or invalid sub-fields from the secure built-in default, so once the
key is present in yml it always yields a complete policy that
overlays the DB value the same way autoMaintain already does.
@dosubot dosubot Bot added the size:S label Jul 1, 2026
@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-01 22:55:11 UTC

3 files · 1 AI reviewer · no blockers · readiness 75/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

Review summary
The change wires `settings.commandAuthorization` into the focus-manifest settings path and deliberately preserves DB policy when the YAML value is not a mapping, which matches the documented overlay behavior and avoids replacing a stricter stored policy with defaults. The tests cover the main reachable paths: valid override, unset inheritance, invalid top-level shapes, and normalizer warnings for invalid nested values. I do not see a correctness break in the visible diff.

Nits — 5 non-blocking
  • nit: `src/signals/focus-manifest.ts:643` carries a long policy explanation inline; consider trimming it to the top-level-shape distinction and leaving the full rationale in the test or issue for readability.
  • nit: `test/unit/focus-manifest.test.ts:1144` does not explicitly assert the cache round-trip through `settingsOverrideToJson`; adding one assertion would lock down the claim that the generic serializer preserves this newly added settings field.
  • Add a focused `settingsOverrideToJson` assertion in `test/unit/focus-manifest.test.ts` for a parsed `commandAuthorization` policy so future serializer narrowing cannot silently drop it.
  • Shorten the parser comment in `src/signals/focus-manifest.ts` to the actionable invariant: object values are normalized and non-object values warn without setting `out.commandAuthorization`.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2268
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:M; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 572 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 572 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Triage stale or unlinked PRs.
  • No action.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. gittensor labels Jul 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.88%. Comparing base (76e9baa) to head (dce7d9a).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2385   +/-   ##
=======================================
  Coverage   95.88%   95.88%           
=======================================
  Files         224      224           
  Lines       25136    25142    +6     
  Branches     9139     9141    +2     
=======================================
+ Hits        24102    24108    +6     
  Misses        421      421           
  Partials      613      613           
Files with missing lines Coverage Δ
src/signals/focus-manifest.ts 99.19% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…e the DB policy

normalizeCommandAuthorizationPolicy falls back to the built-in default
policy (with a warning) when the top-level shape isn't a mapping, but
focus-manifest.ts unconditionally wrote that fallback result into the
parsed settings override. Since the resolver overlays manifest
settings on top of DB settings, a malformed .gittensory.yml value
(e.g. a string instead of an object) silently overwrote a stricter
DB-persisted commandAuthorization policy with the built-in default.

Only apply the normalized policy when the raw value is actually a
mapping; otherwise warn and leave the override unset so the resolver
preserves whatever the DB already has.
@dosubot dosubot Bot added size:M and removed size:S labels Jul 1, 2026
JSONbored added 2 commits July 1, 2026 15:52
… in the example

The example still described the pre-fix behavior (invalid shape falls
back to the built-in default). Update it to match the actual contract:
an invalid top-level shape is ignored with a warning, preserving any
existing DB-stored policy instead of overwriting it.
@JSONbored
JSONbored merged commit 51d283e into main Jul 1, 2026
9 checks passed
@JSONbored
JSONbored deleted the claude/command-authorization-config-parser branch July 1, 2026 22:59
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
@github-actions github-actions Bot mentioned this pull request Jul 2, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

fix(config): settings.commandAuthorization is documented as config-as-code but silently ignored by the parser

1 participant