fix(selfhost): use own-property check for retired config-lint fields#3379
Conversation
unknownTopLevelWarnings classified retired vs unknown top-level manifest fields with `key in RETIRED_FIELD_MIGRATION_WARNINGS`, which walks the prototype chain. A manifest field named like an Object.prototype member (constructor, toString, hasOwnProperty, valueOf, ...) therefore tested true for the inherited property and resolved to the prototype's function instead of a real warning string — corrupting the string[] result (the function serializes to null over JSON) and suppressing the genuine unknown-field warning for that suspicious key. Use Object.prototype.hasOwnProperty.call, matching the sibling recognizedFieldsFor in the same file. Adds a regression test for a constructor-named field.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3379 +/- ##
=======================================
Coverage 93.89% 93.89%
=======================================
Files 283 283
Lines 30573 30574 +1
Branches 11138 11138
=======================================
+ Hits 28705 28706 +1
Misses 1211 1211
Partials 657 657
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-05 06:27:04 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 3 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 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.
|
Summary
unknownTopLevelWarnings()insrc/selfhost/config-lint.tssplits unknown top-level manifest fields into "retired" (with a migration warning) vs genuinely "unknown", using the JavaScriptinoperator:RETIRED_FIELD_MIGRATION_WARNINGSis a plain object literal, soinwalks the prototype chain:"constructor" in obj,"toString" in obj,"hasOwnProperty" in obj,"valueOf" in obj, etc. are alltrue. A manifest with a field named like anObject.prototypemember is therefore misclassified as retired, andRETIRED_FIELD_MIGRATION_WARNINGS["constructor"]resolves to the inheritedObjectfunction, which is pushed into a declaredstring[]. BecauseJSON.stringifydrops function values, that warning serializes tonullover any API — and the genuine"Manifest contains unknown top-level field: constructor."warning is silently suppressed, so the suspicious unknown field disappears from operator-facing output.The sibling
recognizedFieldsForin the same file already does this correctly withObject.prototype.hasOwnProperty.call(...); this call site was the inconsistent one. The fix uses an own-property check. Adds a regression test that lints aconstructor:field and asserts the correct unknown-field warning plus that every warning is a string.No linked issue: issue creation on this repo is restricted to collaborators, and this is a small, self-contained correctness fix (own-property check, matching the sibling in the same file) whose rationale is self-evident from the diff.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
npm run test:ciaggregate ran green plusnpm audit --audit-level=moderate(0 vulnerabilities). The changed lines are exercised by the existing retired/unknown-field tests plus the new constructor-named-field regression test (which fails on the oldinoperator and passes withhasOwnProperty), socodecov/patchis 100% for the diff.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.Pure config-lint logic; it makes operator-facing manifest linting correct for prototype-named fields and exposes no secrets. No auth/CORS/session surface, no API/OpenAPI shape change, no UI.
Notes