Commit Graph

18 Commits

Author SHA1 Message Date
sang-neo03 d12b39cf46 feat(vfs): allow absolute paths under a built-in path policy (#2580)
* feat(vfs): allow absolute paths under a built-in path allowlist

Path flags only accepted paths relative to the working directory, so an
agent passing a full path (typically under /tmp) failed on its first call
and had to retry with a relative one.

Absolute paths are now accepted when they resolve inside a built-in
allowlist: the working directory, /tmp, and ~/files. A built-in denylist
covers system and credential locations and wins over the allowlist,
including over the working directory. Both lists are compiled in and read
no environment variable, flag, or config file, so the effective policy is
fixed by the binary; upgrading is all it takes for the new behavior to
apply.

Containment is decided by file identity (device and inode) alongside the
resolved name, because a single directory has many spellings: APFS folds
U+017F onto "s", so ".sshh" spelled with it opens ~/.ssh, and NTFS and
APFS both compare case-insensitively.

Reads are hardened where the policy applies: O_NOFOLLOW pins the final
component, O_NONBLOCK keeps a FIFO from blocking before it can be
refused, and the opened descriptor is matched against the inspected
object, rejected when it is not a regular file, and rejected when it
carries extra hard links. The relaxed local-input tier used by apps
upload keeps its own contract (symlinks are legitimate arguments there)
and gains the denylist check instead.

Two behaviors are deliberate rather than incidental. Working inside a
denylisted directory now refuses even relative paths, since the denylist
is unconditional. Running as root leaves only the working directory and
/tmp, because the home directory is then /root, itself a deny root.

Existing tests asserted the old "every absolute path is refused"
baseline; they now assert the allowlist. Traversal fixtures escape to the
filesystem root, which stays outside every allowed root on Linux, where
the temp directory that hosts t.TempDir() is /tmp itself.

* fix(vfs): close two paths around the built-in denylist

A "~/..." argument had two readings: validation expanded it to the home
directory, while a caller that keeps the original string — SafeLocalFlagPath
returns it verbatim — opens whatever "~" names in the working directory. A
symlink there carried reads past the denylist, confirmed by reading
/etc/passwd through it. Every interpretation of an argument is now checked,
so the shorthand still reaches ~/files while the literal entry cannot
escape.

With no LARKSUITE_CLI_CONFIG_DIR and no reachable home directory,
core.GetBaseConfigDir keeps credentials in a bare ".lark-cli" resolved
against the working directory, which is an allow root. That fallback is now
mirrored as a deny root, so containers whose home lookup fails do not expose
their stored tokens.

* fix(vfs): enforce hard-link checks across readers

* fix(vfs): stop an output hard link from rewriting a file outside the allowlist

A hard link has no target for name resolution to follow, so a link inside an
allowed root looked like an allowed destination while sharing its inode with a
file outside every root. A caller that truncated the approved name in place
rewrote that outside file: `auth qrcode --output <link>` reported success and
replaced a 43-byte JSON file outside the allowlist with its PNG.

Output validation now refuses an existing target that carries more than one
name, which covers callers that write directly, and auth qrcode commits
through a temp file and a rename, which replaces the directory entry and
leaves the other names alone. Writers already going through FileIO.Save were
never affected, since that path has always committed by rename.

* fix(vfs): give the hard-link refusal a workable recovery hint

The message told the caller to copy the file into an allowed directory, which
answers a question they did not ask: the file that triggers this is normally
already inside one, with every one of its names there too. It now states what
the check actually cannot do — enumerate the other names a file is reachable
by — and offers the step that works, which is to copy the file and use the
copy.

* test(vfs): pick the denylist fixture for the platform under test

Two tests reached for "/etc/passwd" as a denylisted absolute path. That path
is not absolute on Windows, so one test met the foreign-path rejection instead
of the denylist it was asserting, and the other saw the path joined to the
working directory and no rejection at all. Both now ask for a deny root that
exists on the platform running them — the credential directories under the
account home qualify everywhere — which keeps the denylist covered on Windows
rather than skipping it there.

Verified on Windows 10.0.19045 by running the package's test binary from this
branch and from main: main passed, this branch failed these two, and both pass
after the change. The other packages this branch touches were compared the
same way and their Windows results are identical on both sides.

* fix(vfs): state the hard-link check as the condition it tests

The check read as "bail out unless the target can be inspected", which
nilerr reads as an error swallowed on the way out. It now names the case it
acts on — an existing regular file with more than one name — and the comment
carries what the early return used to imply: a target that cannot be
inspected has no link count to judge, and the write layer reports the real
failure with proper typing.

* docs(vfs): scope the policy's environment claim to what holds

The header promised that neither list accepts runtime input and that no
caller controlling the environment can widen them. Two inputs contradict
that: LARKSUITE_CLI_CONFIG_DIR contributes a deny root, and where the account
database cannot name the running uid, $HOME decides where ~/files points —
reproduced in a container running as an unregistered uid, which wrote into a
directory the environment chose.

The comments now state the preference and its boundary rather than a
guarantee, and record what the boundary costs: a directory named "files"
under the named path, with the home directory itself still outside the
allowlist and every candidate home still carrying the credential deny roots.
The trustedHome note also said the pure-Go lookup falls back to $HOME
silently; it does so only when $USER is set as well, and returns an error
otherwise, which drops the ~/files root instead of moving it.

No behavior change.

* fix(auth): keep the mode of a QR output file that already exists

Committing the QR write by rename fixed a hard link from rewriting a file
outside the allowlist, but it also changed what happens to the target's mode.
A rename installs the temp file's inode, mode included, where the previous
in-place write left the existing file's mode untouched. Overwriting a target
the caller had restricted to 0600 therefore published it as 0644.

The mode now comes from the file already at the path; only a path with
nothing at it takes the default. Verified against main, which preserved 0600
here, and covered by a test that fails when the fixed mode is restored.

* test(sheets): move the csv file-alias tests onto the new path baseline

Merging main brought #2559's tests for the --file → --csv alias, written
against the policy this branch replaces. Two of them fail on it, both because
the verdict they describe moved rather than disappeared.

The out-of-tree case used /tmp, which the allowlist now accepts, so the value
came back as a missing file instead of an out-of-tree one; it now names a path
no allow root can contain. The directory case is refused when the descriptor
is inspected, before a read is attempted, so the message reads "not a regular
file". What the caller sees of both — the flag named, the cause kept, stdin
offered — is unchanged.

That message listed the kinds it refuses and omitted directories, which is how
it reached a directory test reading as a mismatch. It now names them.

* fix(im): let the path policy judge a download target

`+messages-resources-download` refused an absolute --output before the shared
policy saw it, so the flag stayed relative-only after the policy learned to
accept full paths. It is the command behind 99% of a reported 1,189 download
path errors in one week, where 97.2% of first calls passed an absolute path
and every later success had switched to a relative one.

The shape checks are gone. Both call sites already hand the result to
ResolveSavePath, which applies the allowlist, the denylist and symlink
resolution, so refusing a shape here decided nothing the policy would not
decide better — an absolute path is now answered by where it points rather
than by how it is written.

The file-key checks stay, and they are what the batch caller relies on: it
embeds the key in the path, and a key carrying a separator is refused as a
malformed key, so a traversal cannot be built from one. Verified against a
real tenant: /tmp and ~/files now save, while ~/.ssh, /etc and a path outside
every root are still refused.

* test(im): pin the download output contract the policy now decides

The dry-run suite listed an absolute path among the values --output must
refuse. That held while the command rejected the shape itself; now that the
built-in policy decides, /tmp is an allowed root and the path is accepted, so
the case asserted a rule that no longer exists.

It is replaced by the two halves of the real contract: an absolute path
inside an allowed root reaches the request, and a path that resolves outside
every root — a parent escape from this working directory, or a denylisted
directory — is still turned down as a validation error naming --output.

* fix(vfs): hold a relative path to the working directory

Accepting /tmp as an allow root gave a relative path somewhere new to go.
A process whose working directory sits under /tmp — CI runners, containers
and agent sandboxes commonly arrange that — could climb out with "../" and
still satisfy the allowlist, because the sibling it landed in was also under
/tmp. /tmp is world-writable, so that sibling can belong to another user or
another session, and the write side commits by rename, which replaces an
existing target unconditionally. The previous policy refused this: it
required every resolved path to stay under the working directory.

Naming a full path and climbing out of the working directory are different
acts and no longer share one verdict. An absolute path is judged by the
allowlist, which is what this branch set out to allow; a relative one has to
resolve inside the working directory, whatever wider root contains it.

The home denylist grows at the same time and for the same reason: the working
directory is an allow root and running from the home directory is ordinary,
so a credential store there is reachable by a relative name unless the list
covers it. It now names the common ones — netrc, git and shell credentials,
kube, docker, azure, gh, gcloud, the language package registries — and the
shell histories, which carry pasted keys as reliably as a credential file.

---------
2026-09-01 22:18:29 +08:00
kiraWangRuilong ac6d7ce4f8 feat: propagate credential metadata (#2305) 2026-08-13 14:42:44 +08:00
liangshuo-1 0d6f2c65d7 fix(im): harden resource downloads with validated ranged streams (#2223) 2026-08-07 17:45:40 +08:00
evandance ebdeda854d feat(extension): present restricted commands as absent and trim skills (#1837) 2026-08-05 01:44:50 +08:00
evandance 40a0a9de66 feat(enhancement): centralize HTTP transport policies (#2021) 2026-08-02 14:55:05 +08:00
sang-neo03 4c1a92caa6 refactor: converge success output through a single Emitter that owns the write (#1899)
* refactor: add output emitter contract and differential harness

Introduce a leaf Emitter in internal/output that composes the existing
output primitives (content-safety scan, envelope, jq, format rendering,
notice) behind a single command-scoped port. The emitter is unwired: no
production caller is migrated, so CLI output stays byte-for-byte unchanged.

A differential test harness drives the real legacy entry points
(RuntimeContext.Out/OutRaw/OutFormat/..., WriteSuccessEnvelope and the
pagination formatter) and asserts byte-identical stdout/stderr plus typed
errors, locking behavior before later slices migrate callers.

* refactor: tighten emitter API and cover pagination with real tests

- split Emitter.Success/PartialFailure and drop EmitOptions.OK so a
  missing ok flag can no longer silently emit ok:false
- give StreamPage its own StreamOptions (format + pretty) instead of
  reusing EmitOptions, making "jq needs aggregation" a compile-time fact
- pin the Emitter jq-error contract (returns error, writes no stderr);
  the caller adapter re-emits the legacy stderr line on migration
- add in-package tests driving the real apiPaginate/servicePaginate over
  a mock transport: multi-page aggregation, empty-result fallback,
  MarkRaw handling, and the business-error raw-response red line

* test: use standard TestFactory harness for pagination tests

Replace the hand-rolled RoundTripper + APIClient construction in the
apiPaginate/servicePaginate tests with cmdutil.TestFactory and its
httpmock.Registry, and isolate LARKSUITE_CLI_CONFIG_DIR to t.TempDir(),
matching the repo's standard HTTP-mocked test convention. Assertions and
coverage (multi-page aggregation, empty-result fallback, MarkRaw, and the
business-error raw-response red line) are unchanged.

* refactor: route success output through the single Emitter port

Migrate the success-output surfaces onto internal/output's Emitter,
byte-for-byte identical (proven by frozen golden diffs and the real
paginate/HandleResponse tests):

- RuntimeContext.Out/OutRaw/OutFormat/OutFormatRaw/OutPartialFailure now
  build an Emitter and call Success/PartialFailure; emit and outFormat are
  removed. An adapter maps the returned error back to the legacy
  outputErrOnce / jq-error stderr / exit-code behavior.
- WriteSuccessEnvelope degrades to a thin Emitter.Success delegate; its 8
  callers are unchanged.
- apiPaginate/servicePaginate stream pages via Emitter.StreamPage; the
  aggregate and business-error raw-response branches are untouched.
- HandleResponse routes its non-JSON structured-response branch through
  Emitter.Success.

Frozen golden fixtures replace the runtime legacy oracles so the
differential harness cannot go self-referential after migration.

* fix: keep _notice on struct payloads in Emitter's unknown-format fallback

printLegacyDataJSON now normalizes via toGeneric first (matching FormatValue), so a struct / named-map payload retains its injected _notice on the unknown-format -> JSON fallback rather than dropping it silently. Add a regression test that fails against the pre-fix path.

* refactor: make the Emitter own write failures and stop mutating inputs

Route every Emitter stdout path through a render-to-buffer-then-copy helper so a marshal/render failure leaves stdout empty and surfaces a typed internal error (with cause), and a stdout write failure is propagated instead of silently swallowed. Leaf writers gain error-returning Write* cores; the legacy Print*/FormatValue wrappers keep their exact behavior for unmigrated callers.

- handleEmitterError now captures every error, not only the jq/safety branches; flip OutRaw's write-error test to assert propagation.
- Clone the map before injecting _notice so a caller's payload is never mutated and an existing _notice is never overwritten.
- Preserve jq's own typed error (validation/api) on a bad expression or runtime failure; only wrap genuine stdout write failures.
- Split tests: normative emitter_contract_test.go vs frozen emitter_legacy_compat_test.go (base SHA recorded, self-update env vars removed).

* fix: satisfy license-header and forbidigo lint on the emitter changes

- Move the base-SHA note below the copyright header in the renamed legacy-compat test so the license-header check sees a valid header at the top.
- Route the leaf wrappers' marshal/format stderr messages through a single legacyStderrf helper (one //nolint:forbidigo) instead of bare os.Stderr, preserving exact legacy behavior for unmigrated direct callers while passing forbidigo; drop the now-unused os imports.

* fix: stop legacy CSV wrappers reporting write failures to stderr

Align FormatAsCSV/FormatAsCSVPaginated and FormatValue/FormatPage's CSV branch with the other leaf wrappers: report only marshal failures, swallow write failures. Previously they emitted a 'csv write error' for the (empty) line and the JSON-fallback write failures that the pre-refactor code ignored, and mislabeled a JSON write failure as a CSV one. Failure-path only; success output is unchanged (golden double-diff still byte-for-byte).
2026-07-21 14:32:47 +08:00
evandance c5b5aece33 refactor: retire legacy error envelopes and enforce typed contract (#1449)
* refactor: retire legacy error envelopes and enforce typed contract

Consolidate all command error reporting onto the typed errs.* contract, remove
the legacy error surface that predated it, and tighten the lint guards so the
contract holds across the whole repository going forward.

Every failure now reaches stderr as one envelope shape: a category, an
optional subtype, a human- and agent-readable message, and a recovery hint,
with invalid parameters listed under `params`. The legacy ExitError envelope,
its constructors, and the boundary bridge that promoted untyped config and
authorization errors are deleted, leaving a single path from error to wire.
Predicate commands keep their silent-exit behavior through a dedicated signal
that carries only an exit code.

Infrastructure paths that still emitted ad-hoc envelopes — flag parsing,
unknown commands and subcommands, plugin and policy guards, confirmation
prompts, and auth/config failures — now classify into the same taxonomy.
Business, API, auth, and config exit codes are preserved; the one behavioral
change is that Cobra usage failures (missing required flag, unknown command,
bad arguments) now emit the typed validation envelope and exit 2, matching the
explicit flag and subcommand guards, instead of Cobra's plain-text exit 1.

Enforcement is repo-wide rather than per-path:
- The errscontract guards run by default everywhere instead of through a
  migration allowlist, so legacy envelopes cannot be reintroduced anywhere.
- errorlint runs across the whole repository: every error wrap must use %w and
  every comparison must use errors.Is/errors.As, so interior wraps stay legal
  but can no longer break the chain the typed boundary relies on.
- The errs-no-bare-wrap guard is keyed by structural prefix instead of an
  explicit per-domain allowlist, so new shortcut domains are covered without
  editing a list. It runs where forbidigo is enabled (the shortcut domains and
  the auth/config/service command groups); repo-wide chain integrity for the
  remaining command paths is carried by errorlint above.

* test: align cli_e2e success assertions to the ok envelope

The api and service success path now emits the {"ok":true} envelope, so the
cli_e2e workflow assertions that still expected the old {"code":0} shape via
AssertStdoutStatus(t, 0) fail once they run with live credentials. Switch those
workflow assertions to AssertStdoutStatus(t, true); the fake-payload helper test
in core_test.go keeps its code-shape assertion.
2026-06-17 19:42:38 +08:00
guokexin.02 4a4c3344c8 fix: align api success envelopes (#1489) 2026-06-17 17:41:48 +08:00
evandance 99e314fe0b feat(errs): typed envelope contract for auth-domain errors (#1135)
Every failure on the authentication, authorization, and configuration
path now surfaces as a typed structured error instead of an ad-hoc
envelope. Users and scripts that consume CLI output get:

  - a fixed nine-category taxonomy on the wire, each mapped to a
    stable shell exit code (authentication/authorization/config = 3,
    network = 4, internal = 5, policy = 6, confirmation = 10)
  - identity-aware detail fields (missing_scopes, requested_scopes,
    granted_scopes, console_url, log_id, retryable, hint) carried
    uniformly on the envelope
  - a single canonical policy envelope at exit 6; the legacy
    auth_error carve-out is retired
  - per-subtype canonical message + hint that preserves Lark's
    diagnostic phrasing and routes recovery to the right actor:
    app developer (app_scope_not_applied), user (missing_scope,
    token_scope_insufficient, user_unauthorized), or tenant admin
    (app_unavailable, app_disabled)
  - wrong app credentials classify as config/invalid_client whether
    surfaced by the Open API endpoint (99991543) or the tenant
    access-token mint endpoint (10003 / 10014), instead of
    collapsing to a transport error or api/unknown
  - local shortcut scope preflight emits the same
    authorization/missing_scope envelope (identity + deterministic
    missing-scope set) used by the post-call permission path, so AI
    consumers read the same structured shape from precheck and from
    server-returned permission denial
  - streaming download/upload failures keep the same network subtype
    split (timeout / TLS / DNS / transport) as the non-stream path
    instead of collapsing every cause to a generic transport failure
  - console_url is carried only on the bot-perspective
    app_scope_not_applied envelope (where the recovery action is
    "developer applies the scope at the developer console"); the
    user-perspective missing_scope envelope drops the field, since
    the only actionable user recovery is `lark-cli auth login --scope`
    and pointing an end user at a console they cannot modify is
    misleading
  - bind workflows (Hermes / OpenClaw / lark-channel) flatten dynamic
    Type tags to wire 'config' with the original module name kept
    as a metric label

All 10 typed errors are cause-bearing, nil-safe on .Error() and
.Unwrap(), and defensively clone slice setter inputs. Four lint
rules (CheckNilSafeError / CheckBuilderImmutable / CheckUnwrapSymmetry
/ CheckBuildAPIErrorArms) lock these invariants on migrated paths.
2026-05-30 19:08:41 +08:00
zgz2048 b91f6a23f3 fix: include log_id in base attachment media errors (#1133) 2026-05-28 11:54:18 +08:00
evandance fe72e41fb2 feat(errs): add structured CLI error contract (#984)
Introduce a typed error contract framework for lark-cli so in-process
Go callers can branch via errors.As(&errs.XxxError{}) and shell scripts,
AI agents, and protocol adapters can branch on stable JSON type/subtype
fields instead of regex-parsing free-form messages.

Adds:
- Canonical taxonomy under errs/ (9 categories + typed Error structs
  embedding a shared Problem, RFC 7807-aligned)
- Centralized Lark code metadata + identity-aware BuildAPIError dispatch
- Typed JSON envelope writer alongside the legacy envelope writer
- MCP / OAuth (RFC 6750 Bearer) projection adapters
- Five CI lint guards preventing ad-hoc taxonomy drift

Backward compatibility: legacy *output.ExitError producers (ErrAPI,
ErrWithHint, Errorf, ErrBare) and business shortcuts that use them
continue to render the legacy envelope unchanged. SecurityPolicyError
wire format and exit code are preserved via a carve-out; taxonomy
migration is deferred to PR 2. Domain-specific business migration is
staged across PR 3+.

Framework-direct paths now return typed *errs.*Error: ErrAuth /
ErrValidation / ErrNetwork emit category literals on the wire
(authentication / validation / network), *core.ConfigError is promoted
at the cmd/root boundary with exit code aligned from 2 to 3, and Lark
API permission denials classified by BuildAPIError exit 3.

At the SDK boundary, WrapDoAPIError preserves any already-classified
error (legacy *output.ExitError or typed *errs.*) so output.ErrAuth
from missing credentials surfaces with the auth category and exit 3
intact instead of being downgraded to a network error. Policy responses
classified by BuildAPIError (codes 21000 / 21001) extract challenge_url
and the canonical hint from the response body, matching what the
auth transport already surfaces at the HTTP layer; non-https
challenge URLs are dropped.

First PR in the feat/error-contract-* series.
2026-05-26 11:42:33 +08:00
sang-neo03 ddf6f0cb7d feat(pagination): preserve pagination state on truncation and natural… (#659)
* feat(pagination): preserve pagination state on truncation and natural end

* feat(pagination): drop page_token from merged output to reflect aggregate view
2026-04-25 17:54:52 +08:00
MaxHuang22 600fa50517 feat: add configurable content-safety scanning (#606)
* feat(contentsafety): add extension interface layer with Provider, Alert, and registry

Change-Id: Ibeac6366c7201293057bc3b063f75ac34565bcd5

* feat(contentsafety): add normalize utility for JSON type conversion

Change-Id: I7d4729a5ddcab2553abc110f8f6ecc88435ae921

* feat(contentsafety): add tree walker and regex scanner

Change-Id: I215dad7cf3072711d05e45f7d384162e1f8752d4

* feat(contentsafety): add config loading with lazy creation, default rules, and allowlist matching

Change-Id: I75e10df28f1f8d4f433cb2b469a0ff317af3bf70

* feat(contentsafety): add regex provider with config-driven scanning and allowlist

Change-Id: I658889b3647cbbbde6881e0c5f7c13887a1eb1d4

* feat(contentsafety): add output core with mode parsing, path normalization, and scan orchestration

Change-Id: I1cb9df75f1a4d176d660e2e7a9561314c3787191

* feat(contentsafety): add ScanForSafety entry point and Envelope alert field

Change-Id: I5fdb311e1c8d983a35a58667970b9fd3ac729a5c

* feat(contentsafety): integrate scanning into shortcut Out() and OutFormat()

Change-Id: I33eef1dba14c8a9bd1998857311bdd611f33b916

* feat(contentsafety): integrate scanning into API/service output paths and register provider

Change-Id: Ic3981db6c546a19eadea095d82175f92f4783bec

* fix(contentsafety): emit stderr notice when lazy-creating default config

Change-Id: Ia2491f7a17caceea3125ff9fb58d750dc196d7e7

* style: gofmt factory_default and exitcode

Change-Id: I86c5afdfbbdb68d8137f0ca09ef3b5a1139f4b4e

* fix(contentsafety): vfs for config I/O, mutex for lazy-create, sort matched rules, emit warn on --output path

Change-Id: Ib4982cd54e1bfe0580a0eb03368e6ca818304e1b

* fix(contentsafety): isolate scan goroutine errOut to prevent race on timeout

Change-Id: Ia5a770d7387ba6d3b7fa318fc5f1384214ea10b7

* fix(contentsafety): deep-normalize typed slices so scanner can walk shortcut data

Change-Id: I641e89113d1a2f2285ac6109bd3d7264f5845ea7

* fix(contentsafety): file perms 0600/0700, no result mutation, timeout test, scanTimeout comment

Change-Id: Ie45a2e365ee7098e214e94f8871026cc12029d83
2026-04-23 17:18:29 +08:00
tuxedomm f5a8fbf8f1 refactor: migrate common/client/im to FileIO and add localfileio tests (#322)
* refactor: migrate common/client/im to FileIO and add localfileio tests

- runner resolveInputFlags: replace validate.SafeInputPath + vfs.ReadFile
  with FileIO.Open + io.ReadAll
- SaveResponse: delegate to FileIO.Save + ResolvePath
- cmd/api, cmd/service: pass FileIO to ResponseOptions
- im: replace validate.SafeLocalFlagPath with RuntimeContext.ValidatePath,
  migrate download/upload to FileIO.Save/Open/Stat
- Add path_test.go and atomicwrite_test.go for localfileio
- Add validate_media_test.go for im media flag validation
- Adapt test mocks to fileio.FileInfo interface
2026-04-08 17:31:21 +08:00
Vux a73c9ae27e fix: improve raw API diagnostics for invalid or empty JSON responses (#257)
- Add internal/client/api_errors.go with WrapDoAPIError and WrapJSONResponseParseError to classify JSON decode issues vs generic network errors
- Route cmd/api DoAPI errors and HandleResponse JSON parse errors through the new helpers
- Add regression tests in cmd/api and internal/client

Related: https://github.com/larksuite/cli/issues/215
2026-04-08 14:28:02 +08:00
liangshuo-1 8db4528269 feat: add strict mode identity filter, profile management and credential extension (#252)
* feat: add strict mode identity filter, profile management and credential extension

Port changes from feat/strict-mode-identity-filter_3 branch:
- Add strict mode for identity filtering and configuration
- Add profile management commands (add/list/remove/rename/use)
- Add credential extension framework (registry, env provider)
- Add VFS abstraction layer
- Refactor factory default and client options
- Update shortcuts to use new credential and validation patterns

Change-Id: I8c104c6b147e1901d94aefcefe35a174932c742b
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: go mod tidy

Change-Id: I0f610ccea6bc874248e84c24770944a3071dcc57
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: fix test failures from credential provider migration

- Remove unused TAT stub registrations in api and service tests
  (CredentialProvider manages tokens, SDK no longer calls TAT endpoint)
- Update strict mode integration test: +chat-create now supports user
  identity, so it should succeed under strict mode user

Change-Id: Iab51c2e12a97995e0b95dcd71df212d2d1f76570
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: migrate remaining os calls to internal/vfs

Replace direct os.Stat/Open/MkdirAll/OpenFile/Remove/ReadDir/UserHomeDir
with vfs equivalents in shortcuts/minutes, shortcuts/drive, and
internal/keychain. Add ReadDir to the vfs interface and OsFs implementation.

Change-Id: I8f97e5fb3e1731b4684d276644fcb10fae823067

* fix: resolve gofmt and goimports formatting issues

Change-Id: If61578631f5698f7ca2d9a946ca59753651463fb

* feat: add Flag.Input support for @file and stdin input sources

Add framework-level support for reading flag values from files (@path)
or stdin (-), solving the fundamental problem of passing complex text
(markdown, multi-line content) via CLI arguments where shell escaping
breaks content. Closes #239, fixes #163.

- Add File/Stdin constants and Input field to Flag struct
- Add resolveInputFlags() in runner pipeline (pre-Validate)
- Support @@ escape for literal @ prefix
- Guard against multiple stdin consumers
- Auto-append "(supports @file, - for stdin)" to help text
- Apply to: docs +create/+update --markdown, im +messages-send/+reply
  --text/--markdown/--content, task +comment --content,
  drive +add-comment --content

Change-Id: I305a326d972417542aeadd70f37b74ea456461ef
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: fix pre-existing test failures in task, minutes, and registry

- task/minutes: remove unused tenant_access_token httpmock stubs
  (TestFactory's testDefaultToken provides tokens directly, so the
  HTTP stub was never consumed and failed verification)
- registry: fix hasEmbeddedData() to check for actual services instead
  of just byte length (meta_data_default.json has empty services array)

Change-Id: Ic7b5fc7f9de09137a7254fe1ddf47d24ade40587
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: suppress nilerr lint for intentional nil returns

Both cases intentionally return nil on error for graceful degradation:
- profile list: show friendly message when config is not initialized
- service: skip scope check when token resolution fails

Change-Id: I7285c37277c9b0361a421ab00359244c2cd150b3
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address CodeRabbit review feedback

- runner.go: fail fast when Input is used on non-string flags
- remote_test.go: rename hasEmbeddedData → hasEmbeddedServices
- profile/list.go: add omitempty to optional JSON fields
- service.go: surface context cancellation errors in scope check

Change-Id: I7072d41f8c711b4b37c542e32dfd8150f42b13c0
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: tighten credential resolution and profile flows

Change-Id: I83f6d424540eab9b1708944b9b6e26e8477cc60d

* refactor: centralize identity hint resolution

Change-Id: I38d5f98160b92adb62dc929ae73697ae5b3d64f8

* fix: surface unverified extension identities

Change-Id: Ia86d9bd19add9010176339ec4cc89deb033f5b4f

* fix: honor runtime credential sources in config views

Change-Id: I40b2ffedc5c1db5e08e86b9472ea2b84fa02bb29

* fix: prefer runtime values in config show commands

Change-Id: I5663a53e147577f0f1f533f67d12bea504e6b839

* Revert "fix: prefer runtime values in config show commands"

This reverts commit 4f9db3a227.

* Revert "fix: honor runtime credential sources in config views"

This reverts commit b3bfd526c5.

* fix: harden profile flows and credential boundaries

Change-Id: Ica61cd2730a639f71516cb1b237a639cb6511f7a

* fix: optimize profile and config inspection for agents

Change-Id: I19c368102f19654952638180ab947788a6971563

* refactor: unify credential env contracts

Change-Id: I0ff2c0a650ea53589a0626333e8f6e628ef10a54

* docs: expand AGENTS guidance

Change-Id: I289027dfd364c92205012feef6f05037066c035b

* fix: resolve regression bugs found during PR #252 review

- im: fix double SafeInputPath in resolveLocalMedia → uploadImageToIM/
  uploadFileToIM chain that rejected all local image/file uploads
- credential: stop writing plain-text warnings to stderr, preserving
  JSON envelope contract for AI agent consumers
- profile add: reject duplicate app-id to prevent keychain credential
  collisions across profiles
- profile rename: exclude self when checking name uniqueness so renaming
  to own appId works correctly
- config: replace bare fmt.Errorf with output.Errorf in save-failure
  paths (default_as, strict_mode ×2, profile add)
- factory: remove unused resolveDefaultAs method (lint)

Change-Id: I6aa0d064414016f367f1edb08dd0604adf7bf13d
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove flaky TestColdStart_UsesEmbedded (race in registry)

The test triggers a data race: resetInit() writes package globals while
a background goroutine from a previous test may still be reading them.
The embedded-data path is covered by other tests.

Change-Id: I7a0c3bf85a9fb337b9279c9053697f40a0c0a0d4
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: type-strengthen Brand and DefaultAs across credential chain

Replace raw string fields with typed enums for compile-time safety:
- extension/credential: add Brand and Identity named types
- internal/core: AppConfig.DefaultAs and CliConfig.DefaultAs → Identity
- internal/credential: Account.DefaultAs and IdentityHint.DefaultAs → core.Identity

The full data flow is now typed end-to-end:
  extcred.Brand → core.LarkBrand (named-type cast)
  extcred.Identity → core.Identity (named-type cast)

No string intermediaries, no implicit conversions.

Change-Id: I715b3b3f033fcb624010f1af9619e3562740ef08
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: fix gofmt alignment in extension/credential/types.go

Change-Id: Ibfac0703a5a28f3c6ba4a47bf40696028d0f3b90
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove file/stdin input support from task comment content flag

Change-Id: If49704ca4612465a23bd30b755d6e72a35fc2349
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(cmdutil): remove dead code autoDetectIdentity

autoDetectIdentity() is only called from tests, never from production
code. Remove it along with its 3 test cases to reduce surface area
before the upcoming ctx propagation refactor.

Change-Id: I35a188860f17656f3e1fe9874f87f284985ae196

* refactor(cmdutil): add ctx parameter to resolveIdentityHint

Private method resolveIdentityHint now accepts context.Context and
passes it to CredentialProvider.ResolveIdentityHint instead of using
context.Background(). The caller (ResolveAs) still uses
context.Background() temporarily until its own signature is updated.

Change-Id: I14634a4e0dc1d657d56936ba61a7b7a206da8ac4

* refactor(cmdutil): add ctx parameter to ResolveStrictMode

ResolveStrictMode now accepts context.Context and passes it to
CredentialProvider.ResolveAccount instead of using context.Background().

Callers in cobra RunE pass cmd.Context(); callers outside RunE
(cmd/root.go startup, tests) use context.Background() explicitly.

Change-Id: I31be48e548ac5ac5640a65f3bfdde4a53ed1dc7e

* refactor(cmdutil): add ctx parameter to CheckStrictMode

CheckStrictMode now accepts context.Context and forwards it to
ResolveStrictMode. Callers pass cmd.Context() (cobra RunE) or
opts.Ctx (APIOptions/ServiceMethodOptions).

Change-Id: I47888519d4cae8c94054771c32aff075565a8cdc

* refactor(cmdutil): add ctx parameter to ResolveAs

ResolveAs now accepts context.Context as first parameter and forwards
it to ResolveStrictMode and resolveIdentityHint. This completes the
ctx propagation chain: all Factory methods that call
CredentialProvider now receive ctx from cobra cmd.Context().

No more context.Background() calls remain in factory.go for
credential provider operations.

Change-Id: I6d10b6350e3b149470660de3e7855614314e8b29

* test: fix gofmt in cmdutil factory tests

Change-Id: I4a87d5a815b959f14cc4371b73dee4aae106932f

* fix: remove file/stdin input support from im send/reply and drive comment

The Input (file/stdin) feature is not yet ready for these flags:
- im send/reply: --content, --text, --markdown
- drive add-comment: --content

Retained only in doc create/update where markdown from file is essential.

Change-Id: I582b6349528fccb639ad9edc84650cca3b68535c
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: liushiyao <liushiyao.1206@bytedance.com>
2026-04-07 15:21:14 +08:00
MaxHuang22 7baba213bc feat: add --jq flag for filtering JSON output (#211)
* feat: add --jq flag for filtering JSON output across all command types

Add jq expression filtering (--jq / -q) to api, service, and shortcut
commands using gojq. Includes early expression validation, mutual
exclusion checks with --output and non-json --format, pagination+jq
aggregation path, and comprehensive test coverage.

* fix: correct gofmt alignment in jq_test.go struct literal


* fix: downgrade gojq to v0.12.17 to keep Go 1.23 compatibility

gojq v0.12.18 requires Go 1.24, which unnecessarily bumped the project
minimum version. v0.12.17 requires only Go 1.21 and provides the same
jq functionality needed.


* refactor: consolidate jq validation and pagination logic

Extract ValidateJqFlags() and PaginateWithJq() shared functions to
eliminate duplicated jq logic across api, service, and shortcut commands.

* fix: reject --jq for non-JSON responses and propagate shortcut jq errors

- HandleResponse now returns a validation error when --jq is used with
  a non-JSON Content-Type instead of silently falling through to binary save.
- Shortcut runtime jq errors are captured in RuntimeContext.outputErr
  and propagated as the command exit code, matching api/service behavior.
2026-04-02 18:36:59 +08:00
梁硕 83dfb068ad feat: open-source lark-cli — the official CLI for Lark/Feishu
Change-Id: I113d9cdb5403cec347efe4595415e34a18b7decf
2026-03-28 10:36:25 +08:00