Skip to content

Potential fix for 1 code quality finding - #3

Merged
mobileskyfi merged 1 commit into
mainfrom
ai-findings-autofix/SECURITY.md
May 2, 2026
Merged

Potential fix for 1 code quality finding#3
mobileskyfi merged 1 commit into
mainfrom
ai-findings-autofix/SECURITY.md

Conversation

@mobileskyfi

Copy link
Copy Markdown
Contributor

This PR applies 1/1 suggestions from code quality AI findings.

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
@mobileskyfi
mobileskyfi marked this pull request as ready for review May 2, 2026 05:54
Copilot AI review requested due to automatic review settings May 2, 2026 05:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the repository’s security posture documentation to reflect the current GitHub Security Advisories configuration. It fits into the codebase as a documentation-only correction to the canonical security scanning/status summary in SECURITY.md.

Changes:

  • Mark private vulnerability reporting as enabled in SECURITY.md.
  • Align the security policy text with the existing use of GitHub Security Advisories for private reporting.

@mobileskyfi
mobileskyfi merged commit 6369028 into main May 2, 2026
10 checks passed
mobileskyfi added a commit that referenced this pull request May 5, 2026
… and clarity

---

  Issues worth addressing
  1. S006 has a self-contradiction on the CLI parser — the Proposed defaults table commits to "Hand-written
  argv parsing until at least three real commands exist" but the Open questions section still asks "Should the
   alpha CLI add a framework such as clipanion…?". Either drop the question or mark the default as tentative.
  2. scope: grammar isn't quite consistent with WORKFLOW.md template. WORKFLOW.md specifies extends S###
  (singular). S006 uses extends S002, S003, S004. Either update the template in WORKFLOW.md to permit a
  comma-separated list, or rephrase S006 as extends S002 with the others noted in review_source. Small thing,
  but front-matter values are now meant to be query-able, so it matters more than before.
  3. The "longer --timeout might trigger a different protocol over time" half of answer #2 didn't fully land.
  The new spec text is "reject values above the effective ceiling" — correct for alpha, but the future
  "ceiling exceeded → consider another protocol" idea isn't captured anywhere. Worth a one-liner in
  work/20260504A-typed-core-seams/STATUS.md or in S006's Open questions so it isn't lost.
  4. S007 adds a details URL to every error envelope but doesn't say what happens before that page exists.
  Worth one sentence on the policy (e.g. "details URL is required and may resolve to a generic stub page until
   the per-code page lands") so this doesn't turn into 404 churn the first time the contract is used.
  5. The CDB password disambiguation in S004 is stricter than the answer. Answer #3 allowed --password to
  double as the CDB password if no --cdb-password was given; the new S004 row treats them as fully separate
  settings. Cleaner is probably better — but the answer's intent was "minimize prompts when CDB password
  equals the only password the user has" so it's worth a deliberate yes/no, not silent tightening.
mobileskyfi added a commit that referenced this pull request May 31, 2026
- routeros-errors: split the unknown-path rule so an anchored
  'failure: <object> not found' classifies as command-failed, while
  'no such ...' and bare path-shaped 'not found' stay unknown-path (#4).
- retrieve: the path-existence probe also flattens routeros/unknown-path
  (now produced by mapRouterOsError for native traps) to a single
  validation/unknown-path (#1).
- devices: comment-only edit/set no longer flips the saved-password flag
  (#2) or drops romon-agent (#3); rewrites preserve the prior record's
  field layout instead of forcing the ipAdmin canonical order (#11).
- resolver/target: an explicit well-known native-api port wins over the
  URL scheme, so https + --port 8728 stays plaintext (#8).
- protocols/adapter: REST script execute posts to /execute (baseUrl
  already ends in /rest), fixing the doubled /rest/rest/execute path (#5).
- cli: only treat --help/-h as global when it is the first argument so
  per-command help works; derive command summaries from each command's
  exported metadata (single source of truth) (#7, #9).

Adds regression tests for each fix; updates the adapter test baseUrl to
the realistic /rest form that masked #5.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mobileskyfi
mobileskyfi deleted the ai-findings-autofix/SECURITY.md branch May 31, 2026 07:09
mobileskyfi added a commit that referenced this pull request Jul 18, 2026
…, load-error split (#134 review)

Address PR #181 review findings (Copilot, CodeRabbit, Codex):

- Descriptor shape validation before mapping (Codex #1 / CodeRabbit): an
  old/pre-v1 descriptor with no `descriptorVersion` and no `services` now
  raises a typed `quickchr/unsupported` instead of a raw
  `TypeError: Object.entries(undefined)`. Missing `services` is rejected, not
  silently mapped to `{}`, so a provider contract violation surfaces at
  resolve time rather than a later `--via` lookup. Descriptor now crosses the
  optional-dependency boundary as `unknown` until `validateDescriptor()`.

- Per-key `ServiceEndpointMap` + generic `isEndpointAvailable` (Codex #2 /
  Copilot #1): `map.ssh` keeps its `SshServiceEndpoint` type, so narrowing
  preserves the `auth: SshEndpointAuth`/`batchModes` guarantee Phase 4 gates
  `--via ssh`/`transfer --via sftp` on — no cast/`"batchModes" in` probe.

- Load-error split (Copilot #2): only a genuine module-not-found maps to
  `quickchr/package-unavailable`; an installed package that throws on load is
  a real fault → `quickchr/unsupported` preserving the cause.

- Enrich the four `docs/errors/quickchr/*.md` pages with typical trigger and
  remediation (Codex #3 / actionable-errors instruction).

No-alias decisions for the `expandCdbSelection`/`CdbSelection*` rename
(Copilot #3/#4) declined by the maintainer — nothing depends on centrs as a
library. New unit tests cover every added error path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants