Repository navigation
feat: align live Figma, harden sharing, and establish architecture authority - #824
seonghobae wants to merge 12 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
changed the title
|
@OpenCode review Please review exact current head db59f97 after resolving both current-main conflicts. Snapshot polling now preserves project-epoch stale-response isolation and immediate sequential polling; full frontend tests 33 files/309 tests, typecheck, build, and npm audit pass. The nanoid override is 3.3.18 to clear the HIGH advisory. Re-evaluate this head only; do not transfer predecessor evidence. Current-head checks are queued. |
|
Resolved the only current-main merge conflict at current head e34f69c by normal merge: retained the live-Figma font dependencies and adopted main's @xyflow/react 12.11.3 plus Node 26.7 image pins. Frontend typecheck and focused SharedDiagram/design-token/style-contract tests pass (59). Review/check this exact head. |
|
Validation update for exact HEAD
The required Actions are queued on this exact head ( |
opencode-agent
Bot
added
area: auth
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| def _redact_sensitive_snapshot_fields(data: object) -> dict[str, object]: | ||
| """Project snapshot JSON onto the path-scoped public DTO v1.""" | ||
| if not isinstance(data, dict): | ||
| return {} | ||
|
|
||
| projected: dict[str, object] = {} | ||
| for key in _PUBLIC_SNAPSHOT_V1_ROOT_KEYS: | ||
| if key not in data: | ||
| continue | ||
| public_value = _project_public_value(data[key]) | ||
| if public_value is not _DROP_PUBLIC_VALUE: | ||
| projected[key] = public_value | ||
|
|
||
| for collection, allowed_keys in _PUBLIC_SNAPSHOT_V1_ROW_KEYS.items(): | ||
| rows = data.get(collection) | ||
| if not isinstance(rows, list): | ||
| continue | ||
| projected_rows = [ | ||
| row | ||
| if collection == "schemas" and isinstance(row, str) | ||
| else _project_public_row(row, allowed_keys) | ||
| for row in rows | ||
| ] | ||
| projected[collection] = [ | ||
| row for row in projected_rows if row is not _DROP_PUBLIC_VALUE | ||
| ] | ||
|
|
||
| return projected |
There was a problem hiding this comment.
📝 Info: Public snapshot redaction switched from denylist to strict path-scoped allowlist
_redact_sensitive_snapshot_fields (share.py) was rewritten from a recursive denylist (replacing comment/example_value with *** while preserving all other structure) into a strict allowlist projection. Any root key not in _PUBLIC_SNAPSHOT_V1_ROOT_KEYS and any collection row key not in _PUBLIC_SNAPSHOT_V1_ROW_KEYS is now dropped entirely, and _project_public_value drops any nested object value. This is a fail-closed security improvement, but it is also a behavioral contract change: any legitimate snapshot field that is a dict (e.g. if source is actually an object rather than a scalar) will be silently omitted from the public JSON/exports. The public read-only viewer only relies on relations/columns/pk_columns/fk_edges/indexes, which are all allowlisted, so rendering is unaffected. Worth confirming the real snapshot shape for root keys like source/captured_at matches the scalar assumption so nothing needed by downstream public exports is unexpectedly dropped.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if trust_x_forwarded_for: | ||
| xff = request.headers.get("X-Forwarded-For") | ||
| if xff: | ||
| # Use the right-most value (nearest trusted proxy), trimming whitespace. | ||
| # This is safer than the left-most which can be spoofed by the client. | ||
| ip = xff.split(",")[-1].strip() | ||
| if ip: | ||
| return ip | ||
| forwarded_hops = [part.strip() for part in xff.split(",") if part.strip()] | ||
| if trusted_proxy_hops >= 1 and len(forwarded_hops) >= trusted_proxy_hops: | ||
| return forwarded_hops[-trusted_proxy_hops] | ||
|
|
||
| client = request.client | ||
| if client is None: | ||
| ip = "unknown" | ||
| else: | ||
| ip = client.host or "unknown" | ||
| return "unknown" | ||
| return client.host or "unknown" |
There was a problem hiding this comment.
📝 Info: Rate-limit trusted-proxy-hop fallback aggregates callers under the Docker gateway
resolve_client_ip (rate_limit.py) returns X-Forwarded-For[-trusted_proxy_hops] only when the header contains at least the configured number of hops; otherwise it falls back to the direct peer (client.host). With the production config API_RATE_LIMIT_TRUSTED_PROXY_HOPS=2 and Traefik reached through Docker NAT, any request whose forwarded chain is shorter than 2 hops falls back to the shared bridge-gateway address, collapsing all such callers into one rate-limit bucket. This is the documented safe fallback, not a bug, but it means a misconfigured or non-conforming edge (e.g. a request that bypasses the terminator, or a terminator that does not append a second hop) silently degrades per-client isolation into a single shared bucket. Operators should run the two-address proxy-hop preflight described in the operations runbook before relying on it.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export function toPlainText(value: unknown): PlainText | null { | ||
| if (typeof value !== 'string' || value.length === 0) return null | ||
|
|
||
| return value | ||
| .replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/g, ' ') | ||
| .replace(/[&<>"']/g, (char) => HTML_TEXT_ENTITIES[char]) | ||
| .trim() as PlainText |
There was a problem hiding this comment.
📝 Info: Removal of client-side HTML entity escaping relies on React text rendering
toPlainText (types.ts) and the various sanitizeHtml(...) call sites in App.tsx were removed so display strings (project names, snapshot status, error messages) are now rendered as raw text. Because React escapes text children by default this is not an XSS regression and actually fixes previously double-escaped display (e.g. <Billing... shown literally). The residual risk would only be if any of these values are ever passed to dangerouslySetInnerHTML or concatenated into HTML/attributes elsewhere; a quick audit confirms these values flow only into JSX text nodes and value/textContent, so the change is safe. Noting it because it is a security-adjacent behavior change touching many call sites.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const pollSnapshot = async () => { | ||
| try { | ||
| const s = await getSnapshot(snapshotId as string); | ||
| if (!isCurrent) return; | ||
| setSnapshot(s); | ||
|
|
||
| if (s.status === "succeeded" || s.status === "failed" || s.status === "not_found") { | ||
| if (selectedProjectId) { | ||
| try { | ||
| const snaps = await listSnapshots(selectedProjectId); | ||
| if (isCurrent) setSnapshots(snaps); | ||
| } catch (e) { | ||
| if (isCurrent) setError(String(e)); | ||
| } | ||
| } | ||
| const nextSnapshot = await getSnapshot(snapshotId); | ||
| if (!isCurrent || projectEpochRef.current !== projectEpoch) return; | ||
| setSnapshot(nextSnapshot); | ||
| if (TERMINAL_SNAPSHOT_STATUSES.has(nextSnapshot.status)) { | ||
| void loadProjectSnapshots( | ||
| selectedProjectId, | ||
| projectEpoch, | ||
| "스냅샷 목록을 새로고침하지 못했습니다. 잠시 후 다시 시도해 주세요.", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (isCurrent) { | ||
| timer = window.setTimeout(poll, 1000); | ||
| } | ||
| } catch (e) { | ||
| if (isCurrent) { | ||
| setError(String(e)); | ||
| timer = window.setTimeout(poll, 1000); | ||
| } | ||
| } catch { | ||
| if (!isCurrent || projectEpochRef.current !== projectEpoch) return; | ||
| setError( | ||
| "스냅샷 상태를 확인하지 못했습니다. 잠시 후 다시 시도해 주세요.", | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| poll(); | ||
| timer = window.setTimeout(() => { | ||
| void pollSnapshot(); | ||
| }, 1000); | ||
| }; | ||
|
|
||
| void pollSnapshot(); | ||
|
|
||
| return () => { | ||
| isCurrent = false; | ||
| if (timer !== null) { | ||
| clearTimeout(timer); | ||
| } | ||
| if (timer !== null) window.clearTimeout(timer); | ||
| }; | ||
| }, [selectedProjectId, snapshotId]); | ||
| }, [loadProjectSnapshots, selectedProjectId, snapshotId]); |
There was a problem hiding this comment.
📝 Info: Snapshot polling reschedules on error but stops on terminal/stale states
The rewritten poll loop in App.tsx schedules the next 1s tick after the try/catch when the request either succeeds with a non-terminal status or fails with a still-current error, and returns early (no reschedule) on terminal statuses or when the effect is stale (!isCurrent or projectEpochRef mismatch). This preserves the previous retry-on-error behavior while adding project-epoch guards so a late response after a project switch neither updates state nor keeps polling. The logic is correct, but it is subtle: the reschedule intentionally lives outside the try/catch, so any future early-return added inside the try must remember it also suppresses the next tick.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Repository-facing metadata coordination: this PR is the active |
Current public-surface authority — 2026-09-02The body’s prior exact-head/check snapshot is historical. Current canonical head is All predecessor-head workflow/review evidence is non-transferable after this consolidation. Reacquire exact-head checks/reviews and current base/governance before integration. |
seonghobae
added
the
enhancement
ChatGPT Codex Connector
|
Admission-state correction for exact head Finding: 5 review threads remain unresolved and no current APPROVED review exists. This PR remains Open and is moved to Draft/Proposed. Commits, reviews, threads, and valid delta are preserved. Return it to Ready only after causal repair/non-force reconciliation and fresh exact-head evidence. No bypass, synthetic status/approval, manual rerun, Force Push, review dismissal, or Close is used. |
What changed
/share/{id}viewer to the live Figma authority.no-storeSPA delivery, configurable expiry, owner-only project-scoped revocation, and exact expiry display.implemented_on_main,active_pr,planned,research_only,downstream,deprecated, andout_of_scopeevidence. Governed Forward Engineering remains planned; current export/diff/migration/apply paths are explicitly partial precursors.nanoidpath pinned to 3.3.17.llm-draftboundary, pinned package-facing Apache-2.0 evidence to protectedmain@8dc746920c12988f082e914879d95e13c9693535, and addeddocs/product-technical-gap-baseline.mdwith PRD/TRD/UML/ERD, Context Map, Gap/Action/Status, release, Pages, SBOM/provenance, and third-party notice boundaries.Documentation sufficiency verdict
Before this update the repository was insufficient: it had no canonical Architecture/PRD/TRD/ADR graph/UML/ERD/threat/test/traceability authority set.
At exact head
2d743448f76c28e6c4f28baaab1e789adfb203cdthe set is structurally sufficient for review, with machine-checked links, ADR inventory, lifecycle vocabulary, diagrams, requirement traceability, naming-debt inventory, planned integrity constraints, and explicit residual gaps. It is not evidence that planned Forward Engineering is implemented or that the PR is release-ready.The hourly ChatGPT automation is enabled and externally authoritative for runtime scheduling. ADR-0006 and
docs/automation-contract.mdmirror its non-secret hourly review → fix → verify → PR/check → guarded merge → next-item / no-empty-queue-stop contract without claiming Git history proves execution.Exact-head verification
1b6da5ea3a4ff93fc622433dd112f29b9fb23203: public LLM boundary, mutable license link, and missing Gap baseline contracts all failed.2d743448f76c28e6c4f28baaab1e789adfb203cd: the same three focused document contracts pass locally with warnings treated as errors.docs/index.md@1bb4c018cc1db71b104b7fe74208c08bf321b0ca,docs/product-technical-gap-baseline.md@76bf84ffccfb68b9e1dc9325fc17f4050dc1acbb,backend/tests/test_documentation_contract.py@4d4f58b14cacedfd78256ef92c9a3cdae01f680a,docs/README.md@a1a718f2778871b80143c97012985241ac9ed505.35478205660, Security35478205729, SAST35478205898, and CodeQL35478205728are queued.docs/index.mdreview thread is resolved by the exact-head content; four information-only implementation threads remain open.Evidence and remaining gates
docs/README.mdARCHITECTURE.mddocs/documentation-coverage-matrix.mddocs/traceability-matrix.mddocs/ui-ux/figma-contract.mddesign-qa.mddocs/automation-contract.mdThis remains a draft. Current-head hosted Checks and qualifying independent approval are still required. Queued state is incomplete evidence, and no predecessor validation is transferred. Same-viewport runtime/Figma browser signoff also remains unproven; no historical screenshot is presented as current evidence.