Address review: redact plant owner ids, cache-control, 400, DRY reset
Build image / build-and-push (push) Successful in 15s
Build image / build-and-push (push) Successful in 15s
- Security (2-model, security+correctness): the public payload zeroed Garden.OwnerID but referenced custom plants still carried Plant.OwnerID, leaking the owner's user id to anonymous viewers. Redact plant owner ids too, and assert it in the service test. - Set Cache-Control: no-store on the public read so a capability-URL response isn't held in a shared proxy cache and always reflects the live garden. - createShareLink now 400s on a present-but-malformed JSON body instead of silently enabling (an empty body still means enable-without-rotate). - Extract the transient-editor-state reset into a shared resetTransient store action (3-model finding) and use it from PublicGardenPage. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
This commit is contained in:
+12
-1
@@ -1,6 +1,8 @@
|
|||||||
package api
|
package api
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
|
||||||
"github.com/gin-gonic/gin"
|
"github.com/gin-gonic/gin"
|
||||||
@@ -16,6 +18,10 @@ func (h *handlers) getPublicGarden(c *gin.Context) {
|
|||||||
writeServiceError(c, err)
|
writeServiceError(c, err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// The token is a capability in the URL: keep the response out of any shared
|
||||||
|
// proxy cache, and always serve the live garden (the owner may have edited or
|
||||||
|
// revoked it since the last view).
|
||||||
|
c.Header("Cache-Control", "no-store")
|
||||||
c.JSON(http.StatusOK, full)
|
c.JSON(http.StatusOK, full)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -45,7 +51,12 @@ func (h *handlers) createShareLink(c *gin.Context) {
|
|||||||
var req struct {
|
var req struct {
|
||||||
Rotate bool `json:"rotate"`
|
Rotate bool `json:"rotate"`
|
||||||
}
|
}
|
||||||
_ = c.ShouldBindJSON(&req)
|
// The body is optional (an empty body means enable-without-rotate), but a
|
||||||
|
// present-yet-malformed body is a client error, not a silent enable.
|
||||||
|
if err := c.ShouldBindJSON(&req); err != nil && !errors.Is(err, io.EOF) {
|
||||||
|
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid request body")
|
||||||
|
return
|
||||||
|
}
|
||||||
link, err := h.svc.EnablePublicShareLink(c.Request.Context(), mustActor(c).ID, id, req.Rotate)
|
link, err := h.svc.EnablePublicShareLink(c.Request.Context(), mustActor(c).ID, id, req.Rotate)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeServiceError(c, err)
|
writeServiceError(c, err)
|
||||||
|
|||||||
@@ -38,10 +38,14 @@ func (s *Service) PublicGarden(ctx context.Context, token string) (*FullGarden,
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
// Minimize what an anonymous viewer learns: no role, and don't expose the
|
// Minimize what an anonymous viewer learns: no role, and no owner user id —
|
||||||
// owner's user id (the page renders fine without it).
|
// neither the garden's owner nor the owner of any referenced custom plant
|
||||||
|
// (built-ins already have a nil OwnerID). The page renders fine without them.
|
||||||
full.Garden.MyRole = ""
|
full.Garden.MyRole = ""
|
||||||
full.Garden.OwnerID = 0
|
full.Garden.OwnerID = 0
|
||||||
|
for i := range full.Plants {
|
||||||
|
full.Plants[i].OwnerID = nil
|
||||||
|
}
|
||||||
return full, nil
|
return full, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -84,6 +84,13 @@ func TestPublicShareLink(t *testing.T) {
|
|||||||
t.Errorf("public full = %d objs / %d plops / %d plants, want 1/1/1",
|
t.Errorf("public full = %d objs / %d plops / %d plants, want 1/1/1",
|
||||||
len(full.Objects), len(full.Plantings), len(full.Plants))
|
len(full.Objects), len(full.Plantings), len(full.Plants))
|
||||||
}
|
}
|
||||||
|
// The referenced plant is the owner's custom plant; its OwnerID must be
|
||||||
|
// redacted so an anonymous viewer can't learn the owner's user id.
|
||||||
|
for _, pl := range full.Plants {
|
||||||
|
if pl.OwnerID != nil {
|
||||||
|
t.Errorf("public plant %d leaks OwnerID %d, want nil", pl.ID, *pl.OwnerID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Unknown / blank tokens are masked, never a distinct error.
|
// Unknown / blank tokens are masked, never a distinct error.
|
||||||
for _, bad := range []string{"does-not-exist", " ", ""} {
|
for _, bad := range []string{"does-not-exist", " ", ""} {
|
||||||
|
|||||||
@@ -49,6 +49,12 @@ interface EditorState {
|
|||||||
// pan gesture stands down (both listen on the same svg).
|
// pan gesture stands down (both listen on the same svg).
|
||||||
objectDragging: boolean
|
objectDragging: boolean
|
||||||
setObjectDragging: (v: boolean) => void
|
setObjectDragging: (v: boolean) => void
|
||||||
|
|
||||||
|
// Clear all transient (non-persisted) editor state at once — selection, focus,
|
||||||
|
// armed plant/kind, and any in-flight live geometry — when entering a garden
|
||||||
|
// view fresh (e.g. the public read-only page on mount). The viewport is left
|
||||||
|
// alone; the canvas re-fits it from the loaded garden.
|
||||||
|
resetTransient: () => void
|
||||||
}
|
}
|
||||||
|
|
||||||
export const useEditorStore = create<EditorState>((set) => ({
|
export const useEditorStore = create<EditorState>((set) => ({
|
||||||
@@ -78,4 +84,15 @@ export const useEditorStore = create<EditorState>((set) => ({
|
|||||||
|
|
||||||
objectDragging: false,
|
objectDragging: false,
|
||||||
setObjectDragging: (v) => set({ objectDragging: v }),
|
setObjectDragging: (v) => set({ objectDragging: v }),
|
||||||
|
|
||||||
|
resetTransient: () =>
|
||||||
|
set({
|
||||||
|
selectedId: null,
|
||||||
|
selectedPlantingId: null,
|
||||||
|
focusedObjectId: null,
|
||||||
|
armedPlant: null,
|
||||||
|
armedKind: null,
|
||||||
|
liveObject: null,
|
||||||
|
livePlanting: null,
|
||||||
|
}),
|
||||||
}))
|
}))
|
||||||
|
|||||||
@@ -25,14 +25,7 @@ export function PublicGardenPage() {
|
|||||||
// Start from a clean canvas: if a logged-in user reaches here from their own
|
// Start from a clean canvas: if a logged-in user reaches here from their own
|
||||||
// editor, stale focus/selection state would otherwise dim or highlight things.
|
// editor, stale focus/selection state would otherwise dim or highlight things.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const s = useEditorStore.getState()
|
useEditorStore.getState().resetTransient()
|
||||||
s.setFocusedObject(null)
|
|
||||||
s.select(null)
|
|
||||||
s.selectPlanting(null)
|
|
||||||
s.setArmedPlant(null)
|
|
||||||
s.setArmedKind(null)
|
|
||||||
s.setLiveObject(null)
|
|
||||||
s.setLivePlanting(null)
|
|
||||||
}, [token])
|
}, [token])
|
||||||
|
|
||||||
const objects = useMemo(() => full.data?.objects.map(toEditorObject) ?? [], [full.data?.objects])
|
const objects = useMemo(() => full.data?.objects.map(toEditorObject) ?? [], [full.data?.objects])
|
||||||
|
|||||||
Reference in New Issue
Block a user