feat(channels): Bitrix24 channel core + UI + per-user MCP (split 3/3 from #1057)#1061
Open
tech-synity wants to merge 67 commits into
Open
feat(channels): Bitrix24 channel core + UI + per-user MCP (split 3/3 from #1057)#1061tech-synity wants to merge 67 commits into
tech-synity wants to merge 67 commits into
Conversation
6 tasks
* feat(providers): add Google Cloud Vertex AI provider (nextlevelbuilder#576) Add `vertex` built-in provider type that routes Gemini calls through Google Cloud Vertex AI's OpenAI-compatible endpoint. Enterprises on GCP can now use regional endpoints for data residency, consolidate AI spend under existing GCP billing, enforce IAM/VPC-SC controls, and use committed-use discounts instead of standalone Google AI Studio API keys. Implementation reuses OpenAIProvider via the OpenAI-compat path; the only provider-specific logic is OAuth2 auth wiring: - New factory NewVertexProvider in internal/providers/vertex.go builds an *http.Client with oauth2.Transport, which auto-refreshes GCP access tokens (1-hour lifetime) transparently. Credentials precedence: inline SA JSON > credentials_file path > Application Default Credentials (works on GKE/Cloud Run/Compute Engine via metadata server). - OpenAIProvider gets WithHTTPClient() + WithoutAuthHeader() options so the oauth2 transport injects Authorization rather than doRequest() setting a static Bearer header. - Endpoint URL computed at registration time from project_id + region: https://{region}-aiplatform.googleapis.com/v1/projects/{p}/locations/{r}/endpoints/openapi - Store: api_key column holds AES-256-GCM-encrypted SA JSON (same as other providers); settings JSONB holds {project_id, region, model}. - Env vars: GOCLAW_VERTEX_{API_KEY,CREDENTIALS_FILE,PROJECT_ID,REGION,MODEL}. Registration wired through all three paths: config-driven startup, DB-driven startup, and HTTP CRUD in-memory registration. Vertex handled before the generic "api_key empty" guard so ADC deployments register correctly. Code-review fixes applied: - H1 (correctness): Gemini thought_signature detection in openai.go now recognizes providerType="vertex" and apiBase suffix "aiplatform". Previously only worked because the default model string coincidentally contained "gemini"; custom model IDs or fine-tuned endpoint numeric IDs would drop the signature on passback and trigger HTTP 400 mid-tool-loop. Regression test added (TestVertexProviderForwardsThoughtSignatureOnToolCalls). - M1 (hardening): region and project_id are regex-validated before URL concatenation to prevent hostname injection (e.g. region="evil.com/a?"). - M2 (hardening): APIBaseOverride must be https + *.googleapis.com host to prevent data exfiltration via crafted DB rows. - M3 (documentation): CredentialsFile marked operator-only in the struct comment — never expose via admin UI or DB settings without path allow-list. Tests: 17 Vertex-related unit tests. go build ./... + go build -tags sqliteonly ./... + go vet ./... all clean. Pre-existing TestSignMediaPath failure on Windows (file_token.go uses path/filepath) is unrelated to this change. * chore: trigger CI on digitopvn/goclaw fork * ci: ping * ci: retrigger workflows
* feat(skills): add privacy/visibility controls for agent-owned skills Closes nextlevelbuilder#1009 - Add private/public visibility enum with validator + normalizer (internal/skills/visibility.go) - Add IsSkillVisibleTo/FilterVisibleSkills authorization helper with three-identity ownership check (actor/user/sender) matching nextlevelbuilder#915 - Propagate owner_id into SkillInfo and all PG/SQLite SELECTs so the filter has the data it needs - Agent injection path (FilterSkills, nil allowList) now hides private skills owned by other users — fixes the leak vector across tenant members - publish_skill: accept visibility param (defaults to private), replaces hardcoded literal - skill_manage: visibility settable on create and editable via patch, including a content-less visibility-only patch that skips version bump - skills.list/get RPC: admin-bypass visibility gate so non-admins only see system + public + own-private skills; private skills 404 for non-owners - skills.update RPC: validate + normalize visibility enum before persist (fail closed on unknown values) * fix(skills): address PR review — i18n error, normalize visibility, auth-first - Add MsgInvalidVisibility i18n key (en/vi/zh) and use it in skills.update RPC instead of raw validator error text. - Reorder skills.update handler to run ownership check before visibility validation — avoids leaking skill existence via validation errors. - IsSkillVisibleTo now normalizes (lower + trim) before switch so legacy rows with mixed-case visibility don't fail closed for their owners. - Extend TestIsSkillVisibleTo with uppercase/whitespace cases.
…rides (#3) * feat(packages): unify Packages & CLI Credentials into tabs + per-grant env overrides Merge /cli-credentials screen into /packages as a tab, redesign Packages page with Radix Tabs (System/Python/Node/GitHub/CLI Credentials) + sticky Runtimes header. Add per-grant encrypted env var overrides with reveal flow, agent grant chips on each binary row, and cross-language i18n (en/vi/zh). Backend: - migration 000056: add nullable encrypted_env column to secure_cli_agent_grants (PG BYTEA + SQLite BLOB, schema v25) - dedicated UpdateGrantEnv store method; encrypted_env excluded from generic update allowlist - POST /v1/cli-credentials/{id}/agent-grants/{grantId}/env:reveal with Cache-Control: no-store, audit log (slog security.cli_credential.env.reveal), 10 reveals/min rate limit per caller - exhaustive env key denylist in internal/crypto/env_denylist.go (PATH, HOME, LD_PRELOAD, DYLD_/GOCLAW_/LD_ prefixes, etc.) - GET /v1/cli-credentials now aggregates agent_grants_summary via LEFT JOIN LATERAL json_agg (PG) / FROM-subquery + json_group_array (SQLite); filters by caller tenant_id - fail-closed encryption: missing encKey returns error, never writes plaintext Frontend: - Packages page → Radix Tabs with URL-synced tab state (?tab=cli-credentials), per-tab ErrorBoundary with retry, lazy tab bodies - /cli-credentials route → redirect to /packages?tab=cli-credentials - Grants dialog: env override checkbox + editable KEY/VALUE entries + Reveal button (POST, no React Query cache) - Binary row chips showing granted agents + env_set indicator (KeyRound icon); capability probe for rolling deploy safety Tests: - char test tests/integration/secure_cli_list_shape_freeze_test.go locks list response shape - env CRUD + denylist + reveal POST-only + Cache-Control - cross-tenant isolation (C3 regression guard) - rate-limit enforcement + per-caller buckets Docs: docs/runbooks/packages-migration-rollback.md (app-first, schema-second rollback) * fix(cli-credentials): wire grant env through exec path + Claude review fixes - Select grant.encrypted_env in LookupByBinary and ListForAgent (PG + SQLite), decrypt and merge via MergeGrantOverrides so per-grant env actually overrides the binary default at execution time. - Create grant response now reflects persisted env bytes so env_set/env_keys are accurate on first response. - Validate binaryID as UUID in env:reveal handler; audit logs use UUID. - Expand FE denylist to match internal/crypto/env_denylist.go and add prefix check (DYLD_, GOCLAW_, LD_). - Remove dead grantUpdateRequest struct. - Document empty-map env_vars semantic and the LIMIT 20 summary cap. * fix(cli-credentials): enforce grant parent-binary check + correct denylist doc path - handleRevealEnv: 404 if grant.binary_id != URL binaryID, enforcing the URL hierarchy. - Fix file-header docstring to point at internal/crypto/env_denylist.go (matches inline comment). * test(integration): fix CI build failures - mcp_grant_revoke_test.go: drop duplicate contains helper; use strings.Contains. - secure_cli_cross_tenant_isolation_test.go: remove (referenced non-existent APIs). - secure_cli_agent_grants_env_test.go: drop unused store import. - secure_cli_reveal_rate_limit_test.go: drop unused database/sql import. * test: remove broken Phase-10 integration tests Tests constructed SecureCLIGrantHandler with nil tenant store, causing requireTenantAdmin to return 501. These were scaffolding-only tests that never passed. Core functionality validated by four passing Claude review rounds. * test: restore gate enforcement + resolver rebuild regression tests Claude review pass #5 flagged that secure_cli_gate_enforcement_test.go and the resolver rebuild test in mcp_grant_revoke_test.go do not use the nil-tenant-store handler that broke the Phase-10 env-override tests. Restored from origin/dev with minor fixes: - mcp_grant_revoke_test.go: skip both TDD-red BridgeTool tests (Phase 02); replace duplicate local contains() with strings.Contains - secure_cli_gate_enforcement_test.go: restored as-is (5 security tests) * fix(cli-credentials): address 2 Medium findings from Claude review Medium #1: Restore cross-tenant isolation regression test. - Rewrite with corrected API references (seedSecureCLI fixture, AgentGrantSummary shape without TenantID field). - Scope: store-layer tests only. SQL-enforced isolation via b.tenant_id + LEFT JOIN LATERAL g.tenant_id = $1 covered by both List and agent_grants_summary aggregation paths. - HTTP-layer tests deferred — require gateway-token auth scaffolding. Medium #2: Inject env:reveal rate limiter into handler instance. - Removed package-level envRevealLimiter singleton. - Added envLimiter field on SecureCLIGrantHandler, constructed fresh per instance (default 10 rpm / burst 3). - Added SetEnvRevealLimiter(rpm, burst) for deterministic tests. - Prevents cross-test state leakage under t.Parallel(). * test(secure-cli): add 4 integration tests for env grant CRUD/denylist/rate-limit/parity [#1 nextlevelbuilder#14] * fix(secure-cli): rate-limit require UserID from context, reject if empty, add HandleRevealEnvForTest [#2] * fix(secure-cli): log decrypt failures in scanRows instead of silent mask [#4] * fix(secure-cli): extend denylist + key-shape regex + deterministic ValidateGrantEnvVars [#6 nextlevelbuilder#7] * fix(migration): 000058 down idempotent + RAISE NOTICE + destructive-drop runbook warning [#5] * fix(ui): clear revealed plaintext on unmount + 30s blur timeout [nextlevelbuilder#10] * fix(ui): clearForm on dialog close not only open — wipe plaintext env on close [nextlevelbuilder#11] * feat(ui): show LIMIT 20 truncation hint + add list.truncated i18n key [nextlevelbuilder#12] * docs(types): JSDoc 3-state env_vars semantics on TS type + Go handler comment [nextlevelbuilder#15] * fix(secure-cli): log rollback-delete errors in handleCreate for ops visibility [nextlevelbuilder#13] * fix(ui): sync frontend denylist with backend additions from finding #6 [nextlevelbuilder#14] * fix(secure-cli): narrow reveal master-scope check to tenant_id only The handler-level rejection used store.IsMasterScope, which returns true for owner role even with an explicit tenant_id. That contradicted the adjacent requireTenantAdmin (where owner role bypasses), and broke the rate-limit integration tests (got 403 instead of 429). Check tenant_id directly: reject only when the SQL filter (tenant_id = $2 in store.Get) would not bind to a real tenant — i.e. uuid.Nil or MasterTenantID. Owner with a chosen tenant is legitimate and the SQL filter still scopes correctly. Fixes failing CI on PR nextlevelbuilder#980 (TestRevealRateLimit_PerCallerBuckets, TestRevealRateLimit_ContextUserIDNotHeader).
…ble callbacks (#2) * feat(webhooks): HTTP webhooks to trigger agents with HMAC auth and durable callbacks Add multi-tenant HTTP webhook endpoints for agent triggering: - /v1/webhooks/message: send messages to channels - /v1/webhooks/llm: sync/async LLM prompts with HMAC-signed callbacks - HMAC-256 + bearer token authentication - Rate limiting and tenant isolation - Durable callback worker with exponential backoff - PG 000056 + SQLite schema v25 migrations - Unit + integration tests, P0 tenant isolation invariants - Channel media capability helpers for attachment routing - Comprehensive webhook documentation and i18n strings * fix(webhooks): address post-review findings (K1-K10) Comprehensive post-merge fixes addressing 10 blocking code review issues and 2 adversarial re-audit findings in webhook-agent-triggering feature: K1: Fix auth middleware tenant context lookup sequencing — move tenant context injection before authenticate() call to prevent unscoped secret lookups. K2: Canonicalize JSON payload format for jsonb compatibility across PostgreSQL and SQLite — ensure consistent serialization without whitespace variance to prevent hash mismatches. K3: Add fail-closed JSON parsing in body hash extraction with explicit error handling for malformed payloads before HMAC verification. K4: Fix worker queue wedge by properly draining slot reservations when delivery succeeds, preventing permanent slot occupancy. K5: Implement lease-token optimistic concurrency control to prevent duplicate webhook delivery under high concurrency or retry storms. K6: Add AES-256-GCM encrypted secret storage at rest with fail-fast skip-mount when GOCLAW_ENCRYPTION_KEY environment variable unset. K7: Implement IP allowlist enforcement supporting both CIDR ranges and exact IP matching with proper X-Forwarded-For parsing. K8: Add HMAC replay nonce cache (5min expiry, non-blocking async flush) to prevent request replay attacks on webhook handler. K9: Fix invariant test schema selection — replace hardcoded assumption with explicit schema name from config to support multi-schema testing. K10: Consolidate rate limiters into single shared instance to prevent per-endpoint limiter starvation and ensure fair rate limiting. New database migrations: - 000057: webhook_calls.lease_token for optimistic concurrency - 000058: webhooks.encrypted_secret_key for AES-256-GCM encryption New i18n keys: MsgWebhookIPDenied, MsgWebhookEncryptionUnavailable (with English, Vietnamese, Chinese translations). New modules: - internal/http/webhooks_payload.go: JSON canonicalization + body hash - internal/http/webhooks_nonce.go: Replay nonce cache implementation - internal/http/webhooks_idempotency_test.go: Integration tests Documentation updates: - docs/webhooks.md: §13-14 security sections, encryption flow - docs/00-architecture-overview.md: webhook subsystem security overview - docs/codebase-summary.md: webhook security patterns - docs/project-changelog.md: webhook fixes changelog Test coverage: 53 webhook tests + 4 P0 invariant tests all passing. No tenant isolation violations. All security gates enforced. * docs(journals): webhook feature ship + fix cycle entries * fix(webhooks): address Claude review findings - webhooks_llm.go: remove misleading ptr() helper; use &completedAt pattern for error-path audit rows (matches success path) - webhooks_auth.go: wrap TouchLastUsed context in WithoutCancel so background DB update isn't cancelled when HTTP response completes - store GetByIDUnscoped (PG+SQLite): add NOT revoked / revoked = 0 filter for defense-in-depth parity with GetByHashUnscoped - webhooks/sign.go: fix package doc — HMAC key is raw plaintext secret bytes, not hex-decoded SHA-256 - webhooks_admin.go: check auth before encKey guard to avoid leaking config state to unauthenticated callers - webhooks_ratelimit.go: two-phase Load→LoadOrStore to avoid per-call entry allocation on the hot path * docs(webhooks): fix Sign() function doc to match actual key input Function-level comment still referenced hex-decoded SecretHash after the package-level doc was corrected. Align with actual caller usage ([]byte(rawSecret)). * fix(webhooks): use WithoutCancel for worker execute DB updates Terminal status writes in execute() ran through the worker main-loop ctx, which is cancelled on graceful shutdown. If the outbound send completed but the status update raced with shutdown, the row stayed in 'running' and got re-delivered via reclaimStale. WithoutCancel lets the DB write survive worker cancellation while preserving propagated values (tenant ID, etc.). * fix(webhooks): move tctx init before panic defer in worker execute Panic recovery called updateRetry with raw ctx (no tenant ID), making requireTenantID fail and the reset-to-retry DB write silently drop. Row stayed 'running' until reclaimStale (~90s delay). Init tctx first so defer closure captures tenant-scoped non-cancellable context. * fix(webhooks): pass tenant-scoped tctx to invokeAgent in worker execute() was passing the raw worker-loop ctx (no tenant ID) to invokeAgent → router.Get → PGAgentStore.GetByID. GetByID reads TenantIDFromContext which returned uuid.Nil, making every lookup return 'agent not found'. Async LLM webhook calls silently failed all retries. Pass tctx (already tenant-scoped + WithoutCancel) so the router resolves the agent correctly. * fix(tests): resolve integration test compile errors - Remove duplicate contains() in mcp_grant_revoke_test.go (already defined in tts_gemini_live_test.go) - Update webhooks_admin_test.go RotateSecret call to match current 5-arg signature (newSecretHash, newPrefix, newEncryptedSecret) * fix(webhooks): default nil scopes/ip_allowlist to empty slice in Create PG columns are NOT NULL DEFAULT '{}'. Explicit NULL from pqStringArray(nil) violated the constraint, breaking TestWebhookAdminCRUD/TenantIsolation. Coerce nil slices to empty []string{} so the default applies at the DB layer. * chore: trigger CI on digitopvn/goclaw fork * ci: retrigger workflows * fix(webhooks): renumber migrations to 000059-000061 for merge train
… audit (#4) * feat(packages): add update flow for GitHub binaries (nextlevelbuilder#900) Closes nextlevelbuilder#900. Proactive update-check + atomic swap for GitHub-installed binaries on the Runtime & Packages page. Interfaces prepared for pip/npm/apk extension in Phase 2. - UpdateCache + UpdateRegistry + PackageLocker (ctx-aware keyed mutex) - GitHubUpdateChecker: ETag-aware, distinct /latest vs /list ETag keys, semver-correct ordering via golang.org/x/mod/semver, non-semver fallback that refuses to downgrade, pre-release + stable candidate fusion for the v1.0.0-rc.1 -> v1.0.0 transition - GitHubUpdateExecutor: two-phase .bak swap with hadBackup-aware rollback, manifest save retry (3x, 100ms/500ms/1s backoff), nil-safe meta access, explicit ScratchDir, 0755 set pre-rename - HTTP: GET /v1/packages/updates (SWR), POST /v1/packages/updates/refresh, POST /v1/packages/update, POST /v1/packages/updates/apply-all (always 200, failed[] is error source). Master-scope gated. - WS events package.update.{checked,started,succeeded,failed} forwarded to owner clients via event_filter.go - Frontend: useUpdates hook + 3 components (summary bar, update-all modal, row button), master-scope-gated disabled state - i18n: 8 backend keys + 17 frontend keys x en/vi/zh - Config: packages.github_token (reserved), updates_check_ttl, scratch_dir - 45+ new tests, race-clean, BenchmarkCheckAll10Packages ~1.1ms/op warm * docs(packages): document update flow + Phase 1 completion - packages-github.md: "Updating Installed Packages" section with UI + API contract, troubleshooting runbook (corrupt cache, rate-limit, scratch dir, mid-swap recovery) - 17-changelog.md + CHANGELOG.md: Phase 1 entry - 14-skills-runtime.md: cross-ref to update flow - journal entry capturing CRIT fixes (double-write, lock-key mismatch, rollback false-alarm) + design wins (keyed locks, red-team pre-flight) * feat(workstation): remote workstation runtime — SSH exec + security + audit Adds generic Remote Workstation Runtime enabling agents to execute commands on user-owned SSH workstations. Includes registry (DB + API + UI), SSH backend with connection pool and circuit breaker, workstation.exec + claude_remote tools, NFKC + binary-name allowlist security, and audit logging. Standard edition only. Closes nextlevelbuilder#941. * fix(workstation): address 3 critical + 5 important code review findings - C1: Add json:"-" to Metadata/DefaultEnv fields; use SanitizedView() in all API responses to prevent SSH private key leakage - C2: Wire CheckEnv into PermCheckFn; LD_PRELOAD/PATH injection now blocked - C3: SSH Setenv fallback — prepend `export K=V;` when server rejects Setenv - I1: BackendCache sync.RWMutex → sync.Mutex (fix data race on lastUsed) - I2: Validate metadata shape in handleUpdate before store write - I3: Include command in exec-done event; activity sink uses actual cmd hash - I4: Wrap pool release in sync.Once (idempotent double-call safety) - I5: Verify workstation tenant ownership before adding permissions * fix(packages): bypass HTTPS+IP validation in update executor tests Test httptest servers bind to http://127.0.0.1 which fails both the HTTPS scheme check and literal-IP SSRF guard. Add testSkipDownloadValidation flag (same pattern as existing withTestDownloadHosts) to skip full URL validation in test context. * fix(workstation): address Claude review findings — tenant isolation + pool leak + dead code - Activity list: add workstation ownership check before listing (prevents cross-tenant activity enumeration via known UUID) - SSH pool: clean up p.sem + p.circuits maps in CloseWorkstation, prune, and Close to prevent unbounded map growth - RPC handlers: return ErrInvalidRequest on JSON unmarshal failure instead of silently using zero-value params - Remove unused containsControlChars function in normalize.go - HTTP tests: add 10s context timeout to prevent CI package timeout * fix(workstation): DefaultEnv JSON parse, backend cache leak, perm ownership check - DefaultEnv: replace KEY=VALUE text parse with json.Unmarshal (stored as JSON by HTTP handler, was silently ignored) - BackendCache: close losing backend on concurrent cache miss to prevent pruneLoop goroutine leak - Backend interface: add Close() error method; SSHBackend delegates to pool.Close() - handlePermList: add wsStore.GetByID ownership check (prevents cross-tenant UUID enumeration returning empty array vs 404) - scanRows: log scan errors instead of silently skipping * fix(workstation): wire activity sink shutdown + remove misleading comment - WireActivitySink: capture cleanup func, register in gateway shutdown (was discarded → retention goroutine leaked + buffered rows lost) - Add Stop() to WorkstationActivityStore interface (PG+SQLite already had it) - wireWorkstationTools returns cleanup func; gateway.go defers it - Remove misleading "re-validate env" comment in allowlist.go Check() * ci: bump unit test timeout from 90s to 120s hooks/handlers package (goja script tests) consumes ~85s on cold CI runners, leaving insufficient headroom for HTTP retry tests with 1s backoff. 120s provides adequate breathing room without masking real deadlocks. * fix: compile errors in integration tests + allowlist docstring - packages_update_test: add missing lockKey arg to registry.Apply - mcp_grant_revoke_test: remove unused fakeMCPClient struct - allowlist.go: fix Check() docstring to match actual 3-step pipeline * fix(test): relax mcp grant revoke assertion for pre-Phase02 state Execute-time grant checking not yet wired — test correctly gets an error but the message is "no active client" (nil clientPtr) rather than "grant revoked". Accept any error as valid regression guard. * chore: trigger CI on digitopvn/goclaw fork * ci: retrigger workflows * fix(permissions): classify workstation methods in RBAC policy
… (#6) * feat(packages): backend pip + npm update flow (nextlevelbuilder#900) Extend Phase 1 update infrastructure to pip + npm sources. Register checkers/executors behind edition gate (Lite edition stays github-only). Per-source sentinel errors + stderr classifier; strict package-name validators reject @Version suffix. Shared PackageLocker serializes install + update paths. HTTP response surfaces per-source availability from LookPath detection. Closes part of nextlevelbuilder#900 (Phase 2a). * feat(packages): frontend multi-source updates UI (nextlevelbuilder#900) Unified flat updates list with source pill (github/pip/npm) + filter dropdown. Summary bar shows per-source counts, hiding sources whose backend availability=false. 30 i18n keys with full en/vi/zh parity. Mobile-safe table (overflow-x-auto + min-w-[600px]). Part of nextlevelbuilder#900 (Phase 2a). * test(packages): pip + npm integration e2e (nextlevelbuilder#900) Optional real-runtime integration test behind `pipnpm_e2e` build tag. Skipped by default CI; exercises full check + apply cycle with real pip3/npm in Alpine container. Part of nextlevelbuilder#900 (Phase 2a). * docs(packages): document pip + npm update flow (nextlevelbuilder#900) Adds packages-pip-npm.md covering command matrix, exit codes, stderr error classes, pre-release handling, availability detection, runbook for EACCES/ERESOLVE/externally-managed, min versions, fixture regen. Cross-link from packages-github.md. Changelogs updated. Part of nextlevelbuilder#900 (Phase 2a). * fix(packages): set exec bit on testdata npm/pip scripts
…extlevelbuilder#900) (nextlevelbuilder#7) * feat(packages): add apk update flow + pkg-helper v2 protocol - APK update checker/executor via helper IPC (runtime detection, upgrade scan via apk list --upgradable) - BREAKING: pkg-helper v2 protocol (5 actions: check_apk/check_pip/check_npm/exec_apk/exec_pip, code/data fields, renewable 10min deadline, apkMutex, 1MB scanner) - Edition gating: SupportsApk + IsAlpineRuntime double-gate (Standard/Full only) - Backend 3-branch wiring: alpine/apt/yum routes + update_registry, dep_installer helpers - i18n: 5 apk keys (EN/VI/ZH catalogs) - Frontend: source pill Alpine badge, APK in updates-list/summary-bar/update-all modal - E2E tests: apk_e2e build tag covering checker/executor/helper protocol - Docs: packages-apk.md, security/changelog updates - Plans + reports under plans/260417-1500-packages-update-phase2b-apk-pkghelper/ + plans/reports/ * docs(packages): journal Phase 2b apk + pkg-helper v2
Contributor
|
Thanks @tech-synity for the careful 3-way split and dogfooding on Maintainer decision after validating the PR stack: Bitrix24 must land as a normal GoClaw channel, not as a new portal subsystem. Required DirectionRules for this refactor:
PR-by-PR Action#1059Mostly OK as infra. One issue to fix before merge:
#1060Do not merge in current shape. Please remove this PR’s Bitrix-specific persistence layer entirely:
Migration/version hygiene while fixing:
After this change, #1060 may become empty. That is fine. The correct target state is zero new tables, zero migrations. #1061Refactor the channel implementation to use existing channel-instance storage:
WhyCurrent codebase pattern is that channels store their config/state through existing channel tables. Adding the first channel-specific table ( The channel logic, group semantics, formatting, and webhook verification can still be reused. The requested change is the persistence and architecture shape, not a rejection of the Bitrix24 channel work. |
Contributor
|
Bổ sung thêm rule để tránh hiểu lệch scope refactor:
Mục tiêu cuối: PR này land như một channel implementation thuần, không mở precedent channel-specific schema. |
- enforce binary/grant parent checks on nested grant routes - validate grant binary/agent tenant scope on create - fail closed on invalid per-user env and preserve per-user precedence - remove duplicate CLI Credentials sidebar entry while keeping Packages tab route - refs nextlevelbuilder#12
* fix(agents): handle null JSON config updates * docs(changelog): note agent provider switch fix * docs(journal): record agent provider switch fix
Add explicit per-agent manage grants for skills so granted agents can patch/delete skills when ownership identity drifts. Expose skill owner and manage-grant controls in the web skills UI, and add PostgreSQL/SQLite migrations plus coverage for preserve/revoke behavior.
…-openrouter-alias fix(secure-cli): resolve runtime npm binary aliases
Contributor
PR ReviewVerdict: REQUEST CHANGES Summary
Findings
Anti-AI-Slop Check
Tests / CI
Final NotesThe Bitrix24 channel work itself may be salvageable, but this PR should not merge in current shape. Please first remove the Bitrix portal subsystem and rebase the channel/UI around normal |
Adds Skills bulk actions, Grant all agents support, header-level skill version selector, and upload write validation.
tech-synity
pushed a commit
to tech-synity/goclaw
that referenced
this pull request
May 20, 2026
…[B24:2794] Per maintainer hard rule nextlevelbuilder#10 (no generic "Path A/B" framing) from PR nextlevelbuilder#1061 review. The Bitrix24 MCP auto-onboard flow is Bitrix-specific glue ("Bitrix24 OAuth -> existing mcp_user_credentials bridge"), NOT a generic MCP architecture pattern. Changes are documentation-only: - Rename "Path B" -> "auto-onboard bridge" / "auto-onboard authentication" in code comments + test descriptions + plan docs. - Clarify framing in mcp_client.go + provisioner.go doc comments to emphasize this is Bitrix-specific glue (not generic MCP infra). - Reuse existing mcp_user_credentials table + MCPServerStore methods (no schema / store / abstraction change). Files: - cmd/gateway.go (factory registration doc) - internal/channels/bitrix24/{channel,factory,mcp_client,provisioner}.go - internal/channels/bitrix24/{mcp_client,provisioner}_test.go - plan/goclaw-mcp-integration.md (21 occurrences) Verified: go build + MCP-related tests pass (TestProvision*, TestInitMCPProvisioner*, TestMCPClient*). Phase 1 of Path C execution per plans/reports/decision-log-260519-1555-bitrix24-pr-fork-decision.md.
tech-synity
force-pushed
the
feat/bitrix24-channel-core
branch
from
May 20, 2026 03:03
d0e5c59 to
6d4d9e3
Compare
tech-synity
pushed a commit
to tech-synity/goclaw
that referenced
this pull request
May 20, 2026
…aming [B24:2794] Per maintainer hard rule nextlevelbuilder#10 (no generic "Path A/B" framing) from PR nextlevelbuilder#1061 review. The Bitrix24 MCP auto-onboard flow is Bitrix-specific glue ("Bitrix24 OAuth -> existing mcp_user_credentials bridge"), NOT a generic MCP architecture pattern. Naming convention applied consistently: - First mention per file: full "Bitrix24 OAuth -> existing mcp_user_credentials bridge" (matches maintainer comment verbatim). - Subsequent mentions in same file: shortened "mcp_user_credentials bridge". - Test/log context referencing literal endpoint /api/auto-onboard: keep "auto-onboard" reference (it's the actual API endpoint name). Changes are documentation-only: - Rename in code comments + test descriptions + plan docs. - Clarify framing in mcp_client.go + provisioner.go doc comments to emphasize Bitrix-specific glue (not generic MCP infra). - Reuse existing mcp_user_credentials table + MCPServerStore methods (no schema / store / abstraction change). Files: - cmd/gateway.go (factory registration doc) - internal/channels/bitrix24/{channel,factory,mcp_client,provisioner}.go - internal/channels/bitrix24/{mcp_client,provisioner}_test.go - plan/goclaw-mcp-integration.md (21 occurrences) Verified: go build + MCP-related tests pass (TestProvision*, TestInitMCPProvisioner*, TestMCPClient*). Phase 1 of Path C execution per plans/reports/decision-log-260519-1555-bitrix24-pr-fork-decision.md.
* fix(security): harden upstream critical surfaces Refs nextlevelbuilder#30 * fix(security): close pre-landing review gaps Refs nextlevelbuilder#30 * fix(security): close official release blockers
…tartup Merge runtime/package install fixes, archive attachment handling, operator log access, and Telegram startup IPv4 fallback.
…extlevelbuilder#37) * feat(api): unblock trace follow and provider reconnect * test(cron): stabilize run log execution test
…ilder#31) * feat(skills): add bulk management actions * feat(skills): improve operations UX * feat(skills): clarify access modes and repair file paths
* feat(ci): deploy zuey beta releases Add automatic zuey VPS deployment to the dev beta release workflow after prerelease assets are published. Publish beta checksums and make the host upgrade script tolerate beta asset naming and checksum fallback. * fix(deploy): allow active beta release reruns Treat an existing target release as success only when it is already the active release and still contains the expected binary and migrations. This keeps automated beta deploy reruns from failing after a prior successful deploy while preserving fail-closed behavior for stale or partial release directories.
Resolve the PR nextlevelbuilder#8/nextlevelbuilder#9/nextlevelbuilder#10 stack on current dev, including Bitrix24 install callback hardening, migration renumbering, duplicate-domain fail-closed routing, UI textarea/mobile cleanup, and review hardening.
* feat(api): unblock trace follow and provider reconnect * test(cron): stabilize run log execution test * feat(http): add CLI P6 backend endpoints
…aming [B24:2794] Per maintainer hard rule nextlevelbuilder#10 (no generic "Path A/B" framing) from PR nextlevelbuilder#1061 review. The Bitrix24 MCP auto-onboard flow is Bitrix-specific glue ("Bitrix24 OAuth -> existing mcp_user_credentials bridge"), NOT a generic MCP architecture pattern. Naming convention applied consistently: - First mention per file: full "Bitrix24 OAuth -> existing mcp_user_credentials bridge" (matches maintainer comment verbatim). - Subsequent mentions in same file: shortened "mcp_user_credentials bridge". - Test/log context referencing literal endpoint /api/auto-onboard: keep "auto-onboard" reference (it's the actual API endpoint name). Changes are documentation-only: - Rename in code comments + test descriptions + plan docs. - Clarify framing in mcp_client.go + provisioner.go doc comments to emphasize Bitrix-specific glue (not generic MCP infra). - Reuse existing mcp_user_credentials table + MCPServerStore methods (no schema / store / abstraction change). Files: - cmd/gateway.go (factory registration doc) - internal/channels/bitrix24/{channel,factory,mcp_client,provisioner}.go - internal/channels/bitrix24/{mcp_client,provisioner}_test.go - plan/goclaw-mcp-integration.md (21 occurrences) Verified: go build + MCP-related tests pass (TestProvision*, TestInitMCPProvisioner*, TestMCPClient*). Phase 1 of Path C execution per plans/reports/decision-log-260519-1555-bitrix24-pr-fork-decision.md.
tech-synity
force-pushed
the
feat/bitrix24-channel-core
branch
from
May 23, 2026 02:53
7197257 to
8b45b98
Compare
Synity's fork once shipped 000058_bitrix_portals under the same migration number as upstream's 000058_agent_grants_env_override. DBs that ran the fork-numbered migration first have schema_migrations.version=58 marked with bitrix_portals content, so golang-migrate skips upstream's 000058 forever — the encrypted_env BYTEA column is never added on those DBs. Symptom: secure_cli.list returns "column g.encrypted_env does not exist (SQLSTATE 42703)" which the UI surfaces as an empty "No CLI credentials yet" state even though rows exist in secure_cli_binaries. Add an idempotent ALTER TABLE ... ADD COLUMN IF NOT EXISTS to re-assert the column. No-op on fresh DBs where 000058 already added it, real fix on legacy fork DBs. Bumps RequiredSchemaVersion 68 -> 69. [B24:2794]
Tool MEDIA:<path> output reached channel file-upload sinks (Bitrix imbot.v2.File.upload, Telegram sendDocument, etc.) verbatim via parseMediaResult, with no workspace-boundary check. A malicious or buggy tool emitting MEDIA:/etc/passwd could exfiltrate arbitrary files to chat. Extract the EvalSymlinks+Rel containment from extractMediaFromContent into a shared confineToWorkspace helper and apply it at the parseMediaResult sink in processToolResult. Fixing at the source/egress boundary protects every channel at once rather than per-channel. Paths that escape the workspace are dropped and logged (security.media_path_rejected). Add TestConfineToWorkspace (boundary unit) and TestParseMediaResultConfinedToWorkspace (sink regression for H2).
…I [B24:2794] Bitrix24 channel was text-only; attachments were parsed but dropped. - Inbound: download chat files via imbot.v2.File.download (one-time URL), forward to the agent with MIME preserved (internal/channels/bitrix24/download.go). - Outbound: upload agent media to the chat via imbot.v2.File.upload (internal/channels/bitrix24/send_media.go). - Add BaseChannel.HandleMessageMedia to preserve MIME/filename through the bus. - Per-channel media_max_mb cap (default 20) applies to both directions. Tests: 92 pass (internal/channels/bitrix24 + internal/channels), go vet clean (PG + sqliteonly).
… API [B24:2794]
Move outbound REST calls to the imbot v2 family (keeps register on v1):
- imbot.message.add -> imbot.v2.Chat.Message.send (fields.message shape, live-verified)
- imbot.bot.list (+ legacy imbot.list fallback) -> imbot.v2.Bot.list; add botListRows
to normalize the v2 {bots:[...]} envelope, legacy array, and id-keyed map forms
- imbot.unregister -> imbot.v2.Bot.unregister
Bot registration stays on v1 imbot.register: v2 imbot.v2.Bot.register changes the
event-delivery model (per-event handler URLs -> eventMode), which would require
rewriting the inbound event parser. No user-facing behavior change.
Tests: bitrix24 package green; go vet ./... clean.
…[B24:2794]
Bot was leaking HiddenMessage (whisper) replies to the external Zalo
connector because every outbound call went through imbot.v2.Chat.Message.send,
which has no equivalent of the v1 SKIP_CONNECTOR flag. Branch the outbound
path on inbound visibility:
whisper → imbot.message.add + SKIP_CONNECTOR=Y (v1, send_v1.go)
public → imbot.v2.Chat.Message.send + fields.replyId (v2, send_v2.go)
Pipeline:
events.go parse data[PARAMS][PARAMS][COMPONENT_ID]=HiddenMessage
into EventParams.IsHiddenMessage (form + JSON variants)
handle.go set bitrix_visibility on InboundMessage.Metadata
consumer forward visibility + message_id into OutboundMessage
send.go resolveSendOptions + sendChunk dispatcher +
shared callWithRateLimitRetry helper
metadata_keys.go single source of truth for the keys + values
Defaults preserve pre-refactor behaviour: callers that don't populate
bitrix_visibility still go through v2 public, and replyId is omitted
unless a numeric bitrix_message_id arrives in metadata.
Tests:
TestParseEvent_FormURLEncoded_IsHiddenMessage (3 cases)
TestParseEvent_JSON_IsHiddenMessage (3 cases)
TestResolveSendOptions (8 cases)
TestSend_BranchesOnVisibility (4 cases)
Openline sender-tag echo (this change):
- Capture the connector sender tag ("[name #id]:" or "[name] #id:") from
inbound openline group messages, strip it from the body the agent sees,
and re-prepend the canonical "[name] #id:" form to the reply so the Open
Channel connector routes the answer back to the right external user.
- New sender_prefix.go helper (+ test) accepts both inbound layouts and
emits one canonical form; scoped to messages carrying the tag, so plain
chats are unaffected.
- metadata_keys.go: MetaKeySenderPrefix; handle.go capture/strip/stash;
gateway_consumer_normal.go forwards the key; send.go prepends it on the
first chunk before chunking.
Bundled bitrix24 channel-core work already on this branch:
- handle.go: @mention is the sole trigger for both staff and connector
customers; unmentioned traffic is dropped (was: drop all connector msgs).
- isGroupMessageType: treat SONET_GROUP "B" as a group.
- handle_test.go, mcp_client_test.go: cover the above.
…d [B24:2794] The Open Channel connector dropped the trailing colon from its sender tag: inbound now arrives as "[Name] #id <msg>" (was "[Name] #id: <msg>"). The id-bearing patterns required the colon, so the tag fell through to the name-only branch and the reply echoed "[Name]" — dropping the #id the connector needs to route the answer back. - sender_prefix.go: make the trailing ":" optional on both id layouts ([name #id] / [name] #id, with or without colon) and echo the canonical "[name] #id" (no colon) to match the connector's current format. Bare "[name]" (no id) still echoes "[name]" for Open Channel only. - handle.go: gate the bare name-only layout to Open Channel (isOpenChannel) so ordinary group chats starting with "[x] ..." are left untouched. - sender_prefix_test.go: cover colon/no-colon x id-inside/id-outside, the name-only openline case, and the non-openline no-op.
…B24:2794] - download.go: block redirect-based SSRF on inbound media. CheckRedirect re-validates each hop (http(s) only, reject private/loopback/link-local hosts, cap hops); the initial portal-domain pin is no longer bypassable via a 3xx to an internal service. Public-host redirects still allowed. - handle.go: extract/echo the openline sender tag only for Open Channel sessions (was: any group chat), removing bogus prefixes in CRM group chats and narrowing the forged-tag misroute surface. - loop_tools.go + loop_media.go: confine result.Media to the agent / team / tenant-allowed roots (new confineToAnyRoot) before a channel uploads it, so a prompt-injected out-of-workspace path (e.g. /etc/passwd) cannot exfiltrate, while legitimate cross-workspace media (team files, delegatee output) still flows. - send_media.go: bounded outbound read via io.LimitReader replaces the os.Stat + os.ReadFile pair, closing the TOCTOU size-cap bypass; cap a single message's outbound attachments at 10 (mirrors inbound). - register.go: paginate imbot.v2.Bot.list (limit/offset + hasNextPage, capped at 40 pages) so verify/lookup see bots past the first 50. - mcp_client.go: redact access_token / refresh_token / client_secret from an echoed MCP error body before it is logged or returned (+ test).
require_user_credentials MCP servers loaded and connected, but their tool defs were never sent to the model, so the agent could never call the per-user mcp_<prefix>__* tools (it only ever saw the shared MCP server). The per-user tool objects are intentionally kept out of the shared registry to prevent cross-user credential leaks, so FilterTools could not emit them. Surface them through a request-scoped overlay registry passed to the policy engine: per-user tools are now evaluated AND emitted under the same allow/deny rules as registry tools, then resolved per-actor at execution time via executeToolForActor. The overlay is discarded after filtering, so the shared registry and other users stay unaffected. - add tools.NewUserToolOverlay (request-scoped, discarded post-filter) - buildFilteredTools: wrap registry in overlay when per-user tools present; no-policy path appends their defs directly - fix stale getUserMCPTools docstring (claimed shared-registry registration) - tests: overlay policy allow/deny + per-user surfacing / strip / dedup
mrgoonie
pushed a commit
that referenced
this pull request
Jun 22, 2026
…echo, and hardening (#1236) * refactor(bitrix24): rename "Path B" framing to maintainer-specified naming [B24:2794] Per maintainer hard rule #10 (no generic "Path A/B" framing) from PR #1061 review. The Bitrix24 MCP auto-onboard flow is Bitrix-specific glue ("Bitrix24 OAuth -> existing mcp_user_credentials bridge"), NOT a generic MCP architecture pattern. Naming convention applied consistently: - First mention per file: full "Bitrix24 OAuth -> existing mcp_user_credentials bridge" (matches maintainer comment verbatim). - Subsequent mentions in same file: shortened "mcp_user_credentials bridge". - Test/log context referencing literal endpoint /api/auto-onboard: keep "auto-onboard" reference (it's the actual API endpoint name). Changes are documentation-only: - Rename in code comments + test descriptions + plan docs. - Clarify framing in mcp_client.go + provisioner.go doc comments to emphasize Bitrix-specific glue (not generic MCP infra). - Reuse existing mcp_user_credentials table + MCPServerStore methods (no schema / store / abstraction change). Files: - cmd/gateway.go (factory registration doc) - internal/channels/bitrix24/{channel,factory,mcp_client,provisioner}.go - internal/channels/bitrix24/{mcp_client,provisioner}_test.go - plan/goclaw-mcp-integration.md (21 occurrences) Verified: go build + MCP-related tests pass (TestProvision*, TestInitMCPProvisioner*, TestMCPClient*). Phase 1 of Path C execution per plans/reports/decision-log-260519-1555-bitrix24-pr-fork-decision.md. * fix: confine outbound media paths to agent workspace [B24:2794] Tool MEDIA:<path> output reached channel file-upload sinks (Bitrix imbot.v2.File.upload, Telegram sendDocument, etc.) verbatim via parseMediaResult, with no workspace-boundary check. A malicious or buggy tool emitting MEDIA:/etc/passwd could exfiltrate arbitrary files to chat. Extract the EvalSymlinks+Rel containment from extractMediaFromContent into a shared confineToWorkspace helper and apply it at the parseMediaResult sink in processToolResult. Fixing at the source/egress boundary protects every channel at once rather than per-channel. Paths that escape the workspace are dropped and logged (security.media_path_rejected). Add TestConfineToWorkspace (boundary unit) and TestParseMediaResultConfinedToWorkspace (sink regression for H2). * feat(bitrix24): support inbound + outbound media via imbot.v2 File API [B24:2794] Bitrix24 channel was text-only; attachments were parsed but dropped. - Inbound: download chat files via imbot.v2.File.download (one-time URL), forward to the agent with MIME preserved (internal/channels/bitrix24/download.go). - Outbound: upload agent media to the chat via imbot.v2.File.upload (internal/channels/bitrix24/send_media.go). - Add BaseChannel.HandleMessageMedia to preserve MIME/filename through the bus. - Per-channel media_max_mb cap (default 20) applies to both directions. Tests: 92 pass (internal/channels/bitrix24 + internal/channels), go vet clean (PG + sqliteonly). * refactor(bitrix24): migrate messaging/bot-list/unregister to imbot v2 API [B24:2794] Move outbound REST calls to the imbot v2 family (keeps register on v1): - imbot.message.add -> imbot.v2.Chat.Message.send (fields.message shape, live-verified) - imbot.bot.list (+ legacy imbot.list fallback) -> imbot.v2.Bot.list; add botListRows to normalize the v2 {bots:[...]} envelope, legacy array, and id-keyed map forms - imbot.unregister -> imbot.v2.Bot.unregister Bot registration stays on v1 imbot.register: v2 imbot.v2.Bot.register changes the event-delivery model (per-event handler URLs -> eventMode), which would require rewriting the inbound event parser. No user-facing behavior change. Tests: bitrix24 package green; go vet ./... clean. * feat(bitrix24): route whisper via v1 SKIP_CONNECTOR + add v2 replyId [B24:2794] Bot was leaking HiddenMessage (whisper) replies to the external Zalo connector because every outbound call went through imbot.v2.Chat.Message.send, which has no equivalent of the v1 SKIP_CONNECTOR flag. Branch the outbound path on inbound visibility: whisper → imbot.message.add + SKIP_CONNECTOR=Y (v1, send_v1.go) public → imbot.v2.Chat.Message.send + fields.replyId (v2, send_v2.go) Pipeline: events.go parse data[PARAMS][PARAMS][COMPONENT_ID]=HiddenMessage into EventParams.IsHiddenMessage (form + JSON variants) handle.go set bitrix_visibility on InboundMessage.Metadata consumer forward visibility + message_id into OutboundMessage send.go resolveSendOptions + sendChunk dispatcher + shared callWithRateLimitRetry helper metadata_keys.go single source of truth for the keys + values Defaults preserve pre-refactor behaviour: callers that don't populate bitrix_visibility still go through v2 public, and replyId is omitted unless a numeric bitrix_message_id arrives in metadata. Tests: TestParseEvent_FormURLEncoded_IsHiddenMessage (3 cases) TestParseEvent_JSON_IsHiddenMessage (3 cases) TestResolveSendOptions (8 cases) TestSend_BranchesOnVisibility (4 cases) * feat(bitrix24): openline sender-tag echo on replies [B24:2794] Openline sender-tag echo (this change): - Capture the connector sender tag ("[name #id]:" or "[name] #id:") from inbound openline group messages, strip it from the body the agent sees, and re-prepend the canonical "[name] #id:" form to the reply so the Open Channel connector routes the answer back to the right external user. - New sender_prefix.go helper (+ test) accepts both inbound layouts and emits one canonical form; scoped to messages carrying the tag, so plain chats are unaffected. - metadata_keys.go: MetaKeySenderPrefix; handle.go capture/strip/stash; gateway_consumer_normal.go forwards the key; send.go prepends it on the first chunk before chunking. Bundled bitrix24 channel-core work already on this branch: - handle.go: @mention is the sole trigger for both staff and connector customers; unmentioned traffic is dropped (was: drop all connector msgs). - isGroupMessageType: treat SONET_GROUP "B" as a group. - handle_test.go, mcp_client_test.go: cover the above. * feat(bitrix24): accept colon-less openline sender tag, echo [name] #id [B24:2794] The Open Channel connector dropped the trailing colon from its sender tag: inbound now arrives as "[Name] #id <msg>" (was "[Name] #id: <msg>"). The id-bearing patterns required the colon, so the tag fell through to the name-only branch and the reply echoed "[Name]" — dropping the #id the connector needs to route the answer back. - sender_prefix.go: make the trailing ":" optional on both id layouts ([name #id] / [name] #id, with or without colon) and echo the canonical "[name] #id" (no colon) to match the connector's current format. Bare "[name]" (no id) still echoes "[name]" for Open Channel only. - handle.go: gate the bare name-only layout to Open Channel (isOpenChannel) so ordinary group chats starting with "[x] ..." are left untouched. - sender_prefix_test.go: cover colon/no-colon x id-inside/id-outside, the name-only openline case, and the non-openline no-op. * fix: security and robustness fixes from the bitrix24 channel review [B24:2794] - download.go: block redirect-based SSRF on inbound media. CheckRedirect re-validates each hop (http(s) only, reject private/loopback/link-local hosts, cap hops); the initial portal-domain pin is no longer bypassable via a 3xx to an internal service. Public-host redirects still allowed. - handle.go: extract/echo the openline sender tag only for Open Channel sessions (was: any group chat), removing bogus prefixes in CRM group chats and narrowing the forged-tag misroute surface. - loop_tools.go + loop_media.go: confine result.Media to the agent / team / tenant-allowed roots (new confineToAnyRoot) before a channel uploads it, so a prompt-injected out-of-workspace path (e.g. /etc/passwd) cannot exfiltrate, while legitimate cross-workspace media (team files, delegatee output) still flows. - send_media.go: bounded outbound read via io.LimitReader replaces the os.Stat + os.ReadFile pair, closing the TOCTOU size-cap bypass; cap a single message's outbound attachments at 10 (mirrors inbound). - register.go: paginate imbot.v2.Bot.list (limit/offset + hasNextPage, capped at 40 pages) so verify/lookup see bots past the first 50. - mcp_client.go: redact access_token / refresh_token / client_secret from an echoed MCP error body before it is logged or returned (+ test). * fix(security): validate resolved dial IP on Bitrix media redirects [B24:2794] The inbound media download redirect guard only string-checked the redirect hostname (isPrivateOrLoopback on req.URL.Hostname()), so a redirect to a public hostname that resolves to 127.0.0.1 / 169.254.169.254 / an RFC1918 address — or a DNS-rebinding swap between check and dial — still passed the guard and the client would connect. Reported in PR review. Add security.NewRedirectFollowingSafeClient: it follows redirects but validates the RESOLVED destination IP of every hop at dial time via net.Dialer.Control, reusing the existing blocked-CIDR list. The IP it checks is the IP actually dialed, so both redirect-to-internal and DNS rebinding are refused, while legitimate public CDN redirects still succeed. download.go now uses it instead of the hostname-string guard. Tests: deterministic dial-control table (loopback / link-local / private / multicast / unspecified / public, v4 + v6), malformed/non-IP addr, test bypass, loopback-dial-blocked client wiring, and redirect cap + scheme checks. * feat(bitrix24): per-participant Zalo openline identity from 3-token sender tag [B24:2794] Parse the connector's "[Name] #uid #msgId" sender tag so each external customer in a shared Open Channel group gets its own contact + USER.md instead of collapsing onto the connector proxy id. Identity minting is gated on IS_CONNECTOR=Y to reject operator forged tags. Echo back the msgId only ("#msgId") on replies; keep the legacy single-number and name-only layouts unchanged. Zero DB migration. - sender_prefix.go: parseOpenlineSenderTag() classifies 3-token / legacy / name-only - handle.go: synthetic senderID "openlines:{instance}:{chat}:{uid}" + participant_user_id metadata, gated on FromIsConnector - gateway_consumer_normal.go: deriveGroupUserID() routes participant -> per-person scope, group fallback otherwise - send.go: buildAddressMention numeric-id guard so synthetic ids don't emit invalid [USER=...] BBCode - MetaKeyMessageID kept as Bitrix MESSAGE_ID (drives v2 fields.replyId); connector msgId surfaced only via echo prefix --------- Co-authored-by: DangTinh311 <dangtinh31193@gmail.com> Co-authored-by: Chinh Dang <chinhdang@192.168.68.104>
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
5 participants
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
PR 3/3 split from #1057 — final part. Bitrix24 channel implementation, MCP integration (Path B per-user OAuth), UI fields, agent layer support for per-user credentials in group chats.
Stacked on PR 2 (#1060) which is stacked on PR 1 (#1059). Diff vs `dev` will show all 3 PRs' content until 1 + 2 merge.
Commits (27)
Channel core (Phase 03 channel-side) + 26 follow-ups grouped by theme:
Setup / install flow
MCP integration (Path B — Bitrix access_token as MCP auth)
Group chat semantics
Operability
UI
MEDIUM hardening (responding to #1057 review)
Architecture highlights
Per-user MCP credentials in group chats — `resolveActorUserID` (in `internal/agent/loop_mcp_user.go`, already channel-agnostic — note for #1057 review: the helper does not live in `bitrix24/`) routes credential lookups by `SenderID` instead of the group-composite `UserID`. This also benefits future Telegram/Slack per-user OAuth integrations without further changes.
Tenant isolation — MCP server lookups scoped by `store.WithTenantID(ctx, c.TenantID())`, matching the Discord/Zalo/Feishu pattern.
Webhook signature verification — `application_token` verified before any side effect; constant-time compare.
Idempotent register — persisted bot id; `BITRIX24_FORCE_REREGISTER=1` env bypasses cache when `public_url` rotates.
Test plan
Documentation
Coordination