Generic CREATE/UPDATE & DELETE/CANCEL handlers crash on string-form query target — "Cannot read properties of undefined (reading 'length')

[agustino-lim]

Describe the bug

The generic handlers registered by handle_attachments for every application service crash on ordinary, non-attachment writes when the query's target is expressed as a string (entity name) rather than a { ref: [...] } object.

Two conditions in lib/plugin.js (v3.12.2 and current main) dereference .ref unconditionally:

// before(["CREATE","UPDATE"])  — ~line 492
if (req.query?.INSERT?.into.ref.length > 1) {

// before(["DELETE","CANCEL"])  — ~line 574
if (req.query?.DELETE?.from?.ref.length > 1) {

When into / from is a string (e.g. from INSERT.into('My.Entity') / DELETE.from('My.Entity') — a documented, valid form per capire › cds.ql), .ref is undefined, so .ref.length throws:

TypeError: Cannot read properties of undefined (reading 'length')

Two things make this worse:

• These two blocks have no isAttachmentsEntity / hasAttachmentsComposition guard (unlike every other handler in the impl), so they run for all CREATE/UPDATE/DELETE across the app — including entities that have nothing to do with attachments.
• The code is internally inconsistent: the DELETE line already guards from?. but not ref?.; the INSERT line guards neither into?. nor ref?..

This is distinct from #429 ("Remove wrong req.query assumption"), which fixed the outer req.query being absent. That fix shipped in 3.12.0, but the inner into.ref / from.ref deref remains in 3.12.2 and on main.

Real-world impact: this is deploy-blocking for us. A startup data-replication step writes federated data to local entities using the string-form target (UPSERT(rows).into('<entity name>')); with @cap-js/attachments enabled, the unguarded handler trips on these plain inserts and the boot/deploy fails. We currently work around it with a patch-package patch adding the optional chaining.

To Reproduce

Steps to reproduce the behavior:

1. A CAP (Node.js) project with @cap-js/attachments@3.12.2 enabled (cds.requires.attachments), containing at least one entity with Composition of many Attachments (so the plugin activates) plus any plain entity:

   using { Attachments } from '@cap-js/attachments';
   entity Incidents { key ID: UUID; attachments: Composition of many Attachments; }
   entity Books     { key ID: Integer; title: String; }
   
   service MyService {
     entity Incidents as projection on Incidents;
     entity Books     as projection on Books;
   }

2. Issue a write whose target is the string form, dispatched through the service:

   const srv = await cds.connect.to('MyService');
   await srv.run( INSERT.into('MyService.Books').entries({ ID: 1, title: 'x' }) );
   // and/or
   await srv.run( DELETE.from('MyService.Books').where({ ID: 1 }) );

3. Observe the crash thrown from node_modules/@cap-js/attachments/lib/plugin.js at the INSERT?.into.ref.length line (~492) and, for the delete, the DELETE?.from?.ref.length line (~574):

   TypeError: Cannot read properties of undefined (reading 'length')
   

Expected behavior

A string-form (ref-less) target is valid CQN and should be tolerated. The handler should treat "no .ref / not a multi-segment path" as "not a child-entity write" and simply skip its composition check, rather than throwing:

if (req.query?.INSERT?.into?.ref?.length > 1) { … }
if (req.query?.DELETE?.from?.ref?.length > 1) { … }

[ ] is it a regression issue? — No (present since the composition-limit handlers were introduced; #429 fixed a related but different outer-req.query crash).

Environment

• @cap-js/attachments: 3.12.2
• @sap/cds: 9.x
• Node.js: 22

[github-actions]

👋 Hello @agustino-lim, thank you for submitting this issue. Our team is reviewing your report and will follow up with you as soon as possible.

[eric-pSAP]

Hi @agustino-lim, thank you for reaching out. We will have a look at this and get back to you as soon as we can.

[eric-pSAP]

Hi @agustino-lim, I've tried your reproduction setup and was not able to reproduce the error you found. I also checked and it appears to me that the generic handlers are not crashing when the query's target is expressed as a string since cds automatically converts them into the expected object. Could you perhaps share your stack trace so I can see where the failure is coming from?

In addition, what is the exact @sap/cds version you have?

[agustino-lim]

Hi @eric-pSAP, thanks for looking into this, and sorry for the slow follow-up.

I went back and tried to reproduce it properly this time instead of just reading the code, and you're right, it doesn't crash the way I described. I owe you a correction.

A couple of details first: at the time I opened this we were on @sap/cds 9.9.1 with @cap-js/attachments 3.12.2 (we're on 3.13.0 now). On those versions I couldn't get the into.ref / from.ref lines to throw at all. As you said, cds.ql normalizes a string target to {ref:[...]} before the handler ever sees it, and on the requests where there genuinely is no query, the outer req.query?. short-circuits first. So that line just isn't reachable in normal use. My "string-form target" reproduction steps and the data-federation UPSERT explanation were both wrong, I'd inferred them from reading the code rather than from an actual stack trace.

When I dug into what actually bit us, it turned out to be an older problem. The real crash was on attachments 3.10.0, and it was the req.query being undefined case (some of our internal tooling fires REST-style srv.send({ method, path }) requests that carry no query), where req.query.INSERT / req.query.DELETE were dereferenced without a guard. That's exactly what #429 already fixed in 3.12.0, and we stopped seeing it once we moved past that.

So this can be closed, it's effectively a duplicate of the situation #429 already handled, and the rest of it isn't reproducible. The only thing left is cosmetic: the inner into?.ref?. / from?.ref?. could still be hardened defensively, but it's not an actual defect. Up to you whether that's worth a small follow-up.

Thanks again.

[eric-pSAP]

Hi @agustino-lim,

Thanks for the follow up and I'm glad you were able to solve the issue. Please feel free to reach out again if you have any other issues!